Visitar URL original
feat: add ability to write Range values with JSONStreamWriter by PhongChuong · Pull Request #2498 · googleapis/java-bigquerystorage · GitHub
Skip to content
This repository was archived by the owner on Feb 24, 2026. It is now read-only.

feat: add ability to write Range values with JSONStreamWriter - #2498

Merged
PhongChuong merged 20 commits into
mainfrom
rangeJSONStreamWriter
May 31, 2024
Merged

feat: add ability to write Range values with JSONStreamWriter#2498
PhongChuong merged 20 commits into
mainfrom
rangeJSONStreamWriter

Conversation

@PhongChuong

@PhongChuong PhongChuong commented May 10, 2024 •
edited
Loading

Copy link
Copy Markdown
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 #2481 ☕️

@PhongChuong
PhongChuong requested review from a team and agrawal-siddharth May 10, 2024 21:15
@product-auto-label product-auto-label Bot added size: l Pull request size is large. api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. labels May 10, 2024
@PhongChuong

PhongChuong commented May 10, 2024

Copy link
Copy Markdown
Contributor Author

/gcbrun

1 similar comment
@PhongChuong

PhongChuong commented May 10, 2024

Copy link
Copy Markdown
Contributor Author

/gcbrun

@PhongChuong
PhongChuong requested a review from shollyman May 10, 2024 21:42
@yirutang
yirutang requested a review from GaoleMeng May 13, 2024 16:56
yirutang
yirutang reviewed May 16, 2024
View reviewed changes

@yirutang yirutang 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.

Looks good! Some nits.

Comment thread ...uerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/it/ITBigQueryStorageTest.java Outdated
ServerStream<ReadRowsResponse> stream = client.readRowsCallable().call(readRowsRequest);
for (ReadRowsResponse response : stream) {
Preconditions.checkState(response.hasArrowRecordBatch());
rowCount += response.getRowCount();

@yirutang yirutang May 16, 2024

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.

validate the value?

@PhongChuong PhongChuong May 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added validation.

Comment thread ...uerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/it/ITBigQueryStorageTest.java Outdated

JSONObject row3 = new JSONObject();
JSONObject rangeDate3 = new JSONObject();
rangeDate3.put("start", 18262);

@yirutang yirutang May 16, 2024

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.

test date as a string?

@PhongChuong PhongChuong May 30, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to use string values.

Comment thread ...gquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/JsonToProtoMessageTest.java Resolved
yirutang
yirutang reviewed May 16, 2024
View reviewed changes
Comment thread ...gquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/JsonToProtoMessageTest.java Resolved
@product-auto-label product-auto-label Bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels May 30, 2024
@PhongChuong

PhongChuong commented May 30, 2024

Copy link
Copy Markdown
Contributor Author

/gcbrun

PhongChuong
PhongChuong commented May 31, 2024
View reviewed changes

@PhongChuong PhongChuong left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@yirutang
Thanks for the review. I've updated the PR with the suggestions. PTAL.

Comment thread ...gquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/JsonToProtoMessageTest.java Resolved
Comment thread ...uerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/it/ITBigQueryStorageTest.java Outdated

JSONObject row3 = new JSONObject();
JSONObject rangeDate3 = new JSONObject();
rangeDate3.put("start", 18262);

@PhongChuong PhongChuong May 30, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to use string values.

Comment thread ...uerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/it/ITBigQueryStorageTest.java Outdated
ServerStream<ReadRowsResponse> stream = client.readRowsCallable().call(readRowsRequest);
for (ReadRowsResponse response : stream) {
Preconditions.checkState(response.hasArrowRecordBatch());
rowCount += response.getRowCount();

@PhongChuong PhongChuong May 31, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added validation.

@PhongChuong

PhongChuong commented May 31, 2024

Copy link
Copy Markdown
Contributor Author

/gcbrun

yirutang
yirutang reviewed May 31, 2024
View reviewed changes
Comment thread ...gquerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/JsonToProtoMessageTest.java Resolved
Comment thread ...uerystorage/src/test/java/com/google/cloud/bigquery/storage/v1/it/ITBigQueryStorageTest.java Resolved
@PhongChuong

PhongChuong commented May 31, 2024

Copy link
Copy Markdown
Contributor Author

/gcbrun

@PhongChuong
PhongChuong merged commit a5e62be into main May 31, 2024
@PhongChuong
PhongChuong deleted the rangeJSONStreamWriter branch May 31, 2024 20:06
@release-please release-please Bot mentioned this pull request May 31, 2024
Merged
@release-please release-please Bot mentioned this pull request Dec 15, 2025
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@agrawal-siddharth agrawal-siddharth Awaiting requested review from agrawal-siddharth
@shollyman shollyman Awaiting requested review from shollyman
@GaoleMeng GaoleMeng Awaiting requested review from GaoleMeng
1 more reviewer
@yirutang yirutang yirutang approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. size: xl Pull request size is extra large.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for Range types

2 participants

@PhongChuong @yirutang