-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(bigquery): add GEOGRAPHY type support for BigQuery I/O #36121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add support for BigQuery GEOGRAPHY type which works with Well-Known Text (WKT) format. The change includes: - Adding GEOGRAPHY to type mappings in bigquery_tools and bigquery_schema_tools - Implementing GeographyType logical type in schemas.py - Adding comprehensive tests for GEOGRAPHY type conversion and schema integration
- Add _get_project method to handle project billing in BigQuery source - Update tests to explicitly specify project parameter - Fix geography test data formats and simplify test cases - Add temporary storage location for file load tests
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for the BigQuery GEOGRAPHY type, which is a great addition. The changes are comprehensive, including updates to type mappings, a new logical type for GEOGRAPHY, and extensive integration tests.
My review includes a couple of suggestions for improving code maintainability by reducing duplication and simplifying a test pipeline. Overall, the implementation is solid and the test coverage is excellent. Thank you for this contribution!
|
Assigning reviewers: R: @shunping for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
Reminder, please take a look at this pr: @shunping |
|
R: @unography |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
Can you iterate and get the tests green? |
Failed tests were not related to this PR. Just rebased it. |
Thank you! I know that they are often not related. But we need to just get them green and feel the pain of the flakes, and to be entirely sure. TBH I would like to make it impossible to merge on red without extra hoops to jump through, as I have seen on other repos. That would align incentives well... |
Add support for BigQuery GEOGRAPHY type which works with Well-Known Text (WKT) format. The change includes:
Fixes #31945
Test with Direct Runner:
or
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.