Skip to content
This repository was archived by the owner on Apr 1, 2026. It is now read-only.

perf: Fall back to ordering by bq pk when possible#1350

Merged
TrevorBergeron merged 6 commits intomainfrom
use_pk_always
Feb 3, 2025
Merged

perf: Fall back to ordering by bq pk when possible#1350
TrevorBergeron merged 6 commits intomainfrom
use_pk_always

Conversation

@TrevorBergeron
Copy link
Copy Markdown
Contributor

@TrevorBergeron TrevorBergeron commented Feb 1, 2025

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Feb 1, 2025
@TrevorBergeron TrevorBergeron requested a review from tswast February 1, 2025 02:28
@TrevorBergeron TrevorBergeron marked this pull request as ready for review February 1, 2025 02:29
@TrevorBergeron TrevorBergeron requested review from a team February 1, 2025 02:29
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Feb 3, 2025
tswast
tswast reviewed Feb 3, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@tswast tswast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit on naming, otherwise looks good!

Comment thread bigframes/session/_io/bigquery/read_gbq_table.py Outdated
Comment thread bigframes/session/_io/bigquery/read_gbq_table.py
Comment thread bigframes/session/_io/bigquery/read_gbq_table.py
return index_cols


def get_time_travel_datetime_and_table_metadata(
Copy link
Copy Markdown
Contributor

@tswast tswast Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code?

Copy link
Copy Markdown
Contributor Author

@TrevorBergeron TrevorBergeron Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, dead and now, gone

Comment thread bigframes/session/loader.py Outdated
Comment thread tests/unit/session/test_read_gbq_table.py Outdated
Comment thread tests/unit/session/test_read_gbq_table.py Outdated
tswast
tswast reviewed Feb 3, 2025
View reviewed changes
Comment thread bigframes/session/_io/bigquery/read_gbq_table.py Outdated
@TrevorBergeron TrevorBergeron enabled auto-merge (squash) February 3, 2025 21:56
@TrevorBergeron TrevorBergeron merged commit 3c4abf2 into main Feb 3, 2025
@TrevorBergeron TrevorBergeron deleted the use_pk_always branch February 3, 2025 22:46
@release-please release-please bot mentioned this pull request Feb 3, 2025
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@tswast tswast tswast approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@shobsi shobsi

Labels

api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. size: m Pull request size is medium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TrevorBergeron @tswast @shobsi