10BC0 fix: Fix 'sql' property for null index by TrevorBergeron · Pull Request #844 · googleapis/python-bigquery-dataframes · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@TrevorBergeron
Copy link
Contributor

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 Jul 16, 2024
@TrevorBergeron TrevorBergeron marked this pull request as ready for review July 16, 2024 15:58
@TrevorBergeron TrevorBergeron requested review from a team as code owners July 16, 2024 15:58
@TrevorBergeron TrevorBergeron requested a review from shobsi July 16, 2024 15:58
)
sql = bf_df.sql
assert "ORDER BY" not in sql.upper()
assert "farm_fingerprint" not in sql
Copy link
Contributor

Choose a reason for hiding this comment

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

stronger assert with sql.lower(), or even "FARM_FINGERPRINT" not in sql.upper() for consistency with the previous line?

Copy link
Contributor
@shobsi shobsi left a comment

Choose a reason for hiding this comment

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

left a minor comment

@TrevorBergeron TrevorBergeron merged commit 1b6a556 into main Jul 23, 2024
@TrevorBergeron TrevorBergeron deleted the null_index_sql branch July 23, 2024 19:20
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: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0