Skip to content

Conversation

@tswast
Copy link
Collaborator

@tswast tswast commented Apr 24, 2024

This ensures the cached primary_keys is more likely to be correct, in case the user called ALTER TABLE after we originally cached the snapshot time.

Towards internal issue 335727141 and #631
🦕

This ensures the cached `primary_keys` is more likely to be
correct, in case the user called ALTER TABLE after we originally
cached the snapshot time.
@tswast tswast requested review from a team as code owners April 24, 2024 20:14
@tswast tswast requested a review from chelsea-lin April 24, 2024 20:14
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Apr 24, 2024
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. label Apr 24, 2024
Copy link
Contributor

@chelsea-lin chelsea-lin left a comment

Choose a reason for hiding this comment

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

LGTM

cache: Dict[bigquery.TableReference, Tuple[datetime.datetime, bigquery.Table]],
use_cache: bool = True,
) -> Tuple[datetime.datetime, bigquery.Table]:
cached_table = cache.get(table_ref)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if use_cache=False, we can avoid looking through the cache, though the cache won't be too large and the looking is not too expensive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good observation!

Python does dictionary lookups all the time, even for accessing attributes of objects, so I'm not too worried about it. Will leave as-is to avoid the extra nesting.

@tswast tswast merged commit 3acc494 into main Apr 25, 2024
@tswast tswast deleted the b335727141-snapshot-save-metadata branch April 25, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants