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
[BEAM-11659] Add new schema types to Pub/Sub SQL #13980
[BEAM-11659] Add new schema types to Pub/Sub SQL #13980
Conversation
R: @amaliujia |
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.
LGTM overall, thank you. A couple more minor requests:
- Add test(s) in PubsubTableProviderIT for the new configurations
- Update https://beam.apache.org/documentation/dsls/sql/extensions/create-external-table/#pubsub for the new attribute/payload options (this could be done in a separate PR if you prefer)
I'll run some internal tests to double-check everything
...oogle-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/NestedRowToMessage.java
Show resolved
Hide resolved
.../io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubMessage.java
Show resolved
Hide resolved
Run SQL PostCommit |
...e-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java
Outdated
Show resolved
Hide resolved
Done |
I'm going to bulk update this once the other two PRs are merged. |
Run SQL PostCommit |
Thanks! It looks like the new tests aren't passing though. I think one issue is using BYTES instead of VARBINARY, the other is less clear. Maybe because |
Run SQL PostCommit |
...test/java/org/apache/beam/sdk/extensions/sql/meta/provider/pubsub/PubsubTableProviderIT.java
Outdated
Show resolved
Hide resolved
Run SQL PostCommit |
2 similar comments
Run SQL PostCommit |
Run SQL PostCommit |
3fd4ab4
to
7964527
Compare
Run SQL PostCommit |
1 similar comment
Run SQL PostCommit |
Run SQL PostCommit |
...c/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/kafka/KafkaTableProviderIT.java
Outdated
Show resolved
Hide resolved
82beaad
to
e37fc55
Compare
Run SQL PostCommit |
Run Java PreCommit |
1 similar comment
Run Java PreCommit |
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.
LGTM, thank you! Ran internal tests and they look good.
I can merge when CI is green
Add pubsub handling of byte array payload schemas and ARRAY<ROW<VARCHAR key, VARCHAR value>> attributes schemas.
These enable pure-sql lossless copies to and from Pub/Sub Lite and kafka.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.