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

feat: search statistics - #1616

Merged
chalmerlowe merged 56 commits into
mainfrom
feat-search-statistics
Sep 2, 2023
Merged

feat: search statistics#1616
chalmerlowe merged 56 commits into
mainfrom
feat-search-statistics

Conversation

@chalmerlowe

Copy link
Copy Markdown
Collaborator

Adds a feature to display search statistics in support of the following customer request: https://buganizer.corp.google.com/issues/290679162

Includes several classes to encapsulate the data found here:
https://cloud.google.com/bigquery/docs/reference/rest/v2/Job#searchstatistics

Also includes a function to access the data in the classes.

@chalmerlowe
chalmerlowe requested review from a team and farhan0102July 20, 2023 10:26
@product-auto-labelproduct-auto-labelBot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Jul 20, 2023
@shollyman
shollyman requested review from shollyman and removed request for farhan0102July 20, 2023 15:57

@shollymanshollyman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good start on the basic structure. This is an output-only stat so no need for a setter, but we'll need all the unit testing and a similar IT test to java using a query that invokes the search() sql function.

Comment threadgoogle/cloud/bigquery/job/query.py Outdated
"""docstring"""

stats = self._job_statistics().get("searchStatistics")
return SearchStats.from_api_repr(stats)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

need to handle the not-present case, e.g. if stats is not None:

Comment threadgoogle/cloud/bigquery/job/query.py Outdated

@classmethod
def from_api_repr(cls, stats: Dict[str, Any]):
mode = stats.get("indexUsageMode", "INDEX_USAGE_MODE_UNSPECIFIED")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally, we want to avoid the library injecting values not present in the API response, so returning the default value here may not be more desirable than empty/none.

@product-auto-labelproduct-auto-labelBot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Aug 9, 2023
Comment threadtests/unit/job/test_query_stats.py Outdated
@chalmerlowechalmerlowe added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 23, 2023
@yoshi-kokoroyoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 23, 2023
Comment threadtests/unit/job/test_query_stats.py Outdated
Comment threadtests/unit/job/test_query_stats.py Outdated

@shollymanshollyman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After some more reading, I think the attribute approach is sufficient vs explicit property annotations.

Main concern is about attribute defaults. Typically None is better when users are trying to disambiguate between presence of a value vs the empty value, but happy to talk about this more. Swapping the default impacts typing, unit tests, etc.

Comment threadgoogle/cloud/bigquery/job/query.py Outdated
Comment threadgoogle/cloud/bigquery/job/query.py Outdated
Comment threadgoogle/cloud/bigquery/job/query.py Outdated
Comment threadgoogle/cloud/bigquery/job/query.py Outdated
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigqueryIssues related to the googleapis/python-bigquery API.size: mPull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@chalmerlowe@shollyman@yoshi-kokoro