Skip to content
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

fix: add support for repeated record query parameters #2698

Merged
merged 10 commits into from May 25, 2023

Conversation

obada-ab
Copy link
Member

Add support for array of struct query parameters which are used to query repeated record fields

Fixes #2485 ☕️

Add support for array of struct query parameters which are used to query repeated record fields
@obada-ab obada-ab requested a review from a team as a code owner May 15, 2023 12:55
@obada-ab obada-ab requested a review from Neenu1995 May 15, 2023 12:55
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/java-bigquery API. labels May 15, 2023
@obada-ab obada-ab added the owlbot:run Add this label to trigger the Owlbot post processor. label May 15, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 15, 2023
@obada-ab obada-ab added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 15, 2023
typePb.setArrayType(arrayTypePb);
if (getArrayType() == StandardSQLTypeName.STRUCT) {
List<QueryParameterValue> values =
Objects.requireNonNull(getArrayValues(), "Array of struct cannot be empty");
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this restriction coming from?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is currently no way for users to supply detailed struct types (including the fields) when creating an array of struct query parameter using the QueryParameterValue.array method, which would limit us to the option of peeking at the first record to figure out the struct fields.

Ideally, query parameter types should not be limited to primitives and StandardSQLTypeName, users should create more general/nested types and supply them to QueryParameterValue.array, but this would be a much bigger change.

I think this fix with the limitation would solve most/all use cases until the changes mentioned are made.

@@ -4062,6 +4062,47 @@ public void testStructNamedQueryParameters() throws InterruptedException {
}
}

@Test
public void testRepeatedRecordNamedQueryParameters() throws InterruptedException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also write a test with UNNEST option, since that is the particular use case the customer is using?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will work on it

…don't exist

Make the toTypePb method produce an empty 'ARRAY<STRUCT<>>' type when no values exist an an array of struct, which was the original behaviour.

Add IT tests for UNNESTing struct array and creating empty struct array query parameters.
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 17, 2023
@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner May 17, 2023 12:58
@obada-ab obada-ab added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 17, 2023
@obada-ab
Copy link
Member Author

@Neenu1995 I've made some changes:

  • The toTypePb method now only peeks at struct array values if they exist, otherwise it defaults to generating an empty struct array type
  • Added two additional integration tests:
    • testUnnestRepeatedRecordNamedQueryParameter: tests UNNESTing a struct array query parameter
    • testEmptyRepeatedRecordNamedQueryParameters: tests creating an empty struct array query parameter. Creating the query parameter itself works but executing IN UNNEST for said parameter should result in an exception

Also there's something to note: the toTypePb is only used for query parameters, returned query results only go through toValuePb which wasn't touched in this PR.

@obada-ab obada-ab removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 17, 2023
@Neenu1995
Copy link
Contributor

Neenu1995 commented May 18, 2023

String query = "SELECT string, arrayOfStruct FROM `datasetname.tablename`, "
          + "UNNEST(arrayOfStruct) as TEMP WHERE TEMP IN UNNEST(@states);";

      // states - Parameterized query type of Array of Struct
      // arrayOfStruct - Field of type Array of Struct

This is the exact statement we want to verify. Verify if a named parameter type of Array of struct exists in a struct field.
Please create a table and try querying from there.

QueryParameterValue zipValue = QueryParameterValue.string("1h2j34");
QueryParameterValue numberOfYearsValue = QueryParameterValue.string("3");

Map<String, QueryParameterValue> struct = new HashMap<>();
Copy link
Member Author

Choose a reason for hiding this comment

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

The order of fields inside structs matters when doing comparisons, and HashMap does not maintain insertion order. Because of these reasons, the IN UNNEST(addresses) query doesn't return any results.

I tried replacing HashMap with a LinkedHashMap and using ImmutableMap.of, both options made the IN UNNEST(addresses) query work as intended and return the correct row.

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels May 22, 2023
@snippet-bot
Copy link

snippet-bot bot commented May 25, 2023

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@obada-ab obada-ab added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 25, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 25, 2023
@obada-ab obada-ab added the automerge Merge the pull request once unit tests and other checks pass. label May 25, 2023
@gcf-merge-on-green gcf-merge-on-green bot merged commit 51aff50 into main May 25, 2023
19 checks passed
@gcf-merge-on-green gcf-merge-on-green bot deleted the struct-array branch May 25, 2023 17:18
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label May 25, 2023
gcf-merge-on-green bot pushed a commit that referenced this pull request May 31, 2023
🤖 I have created a release *beep* *boop*
---


## [2.27.0](https://togithub.com/googleapis/java-bigquery/compare/v2.26.1...v2.27.0) (2023-05-30)


### Features

* Add support for session id on TableDataWriteChannel ([#2715](https://togithub.com/googleapis/java-bigquery/issues/2715)) ([42851d8](https://togithub.com/googleapis/java-bigquery/commit/42851d818ee825d7c4141d40d116e1da43c11f14))


### Bug Fixes

* Add support for repeated record query parameters ([#2698](https://togithub.com/googleapis/java-bigquery/issues/2698)) ([51aff50](https://togithub.com/googleapis/java-bigquery/commit/51aff502215d69bd0151030421cd18646c6ead36))


### Dependencies

* Update dependency com.google.api.grpc:proto-google-cloud-bigqueryconnection-v1 to v2.20.0 ([#2720](https://togithub.com/googleapis/java-bigquery/issues/2720)) ([4962cac](https://togithub.com/googleapis/java-bigquery/commit/4962cac8fb3fe8d77a136eaf1b579cd79304acfb))
* Update dependency com.google.apis:google-api-services-bigquery to v2-rev20230506-2.0.0 ([#2707](https://togithub.com/googleapis/java-bigquery/issues/2707)) ([4d2ec07](https://togithub.com/googleapis/java-bigquery/commit/4d2ec0716287e9624949cbcdf6605c127c209be4))
* Update dependency com.google.apis:google-api-services-bigquery to v2-rev20230520-2.0.0 ([#2723](https://togithub.com/googleapis/java-bigquery/issues/2723)) ([5c64797](https://togithub.com/googleapis/java-bigquery/commit/5c64797c603343408849535b2dbf8080cd11ca32))
* Update dependency com.google.cloud:google-cloud-bigquerystorage-bom to v2.37.2 ([#2726](https://togithub.com/googleapis/java-bigquery/issues/2726)) ([052c47a](https://togithub.com/googleapis/java-bigquery/commit/052c47aa43b0f50414db3031914e8a775ae98925))
* Update dependency com.google.cloud:google-cloud-datacatalog-bom to v1.24.0 ([#2721](https://togithub.com/googleapis/java-bigquery/issues/2721)) ([7c357fb](https://togithub.com/googleapis/java-bigquery/commit/7c357fb414d45fde734c09c88ee3023d8d8f5822))
* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.10.1 ([#2713](https://togithub.com/googleapis/java-bigquery/issues/2713)) ([744e83a](https://togithub.com/googleapis/java-bigquery/commit/744e83a3da5323bc2cff2bcc6368a3eec39f392e))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
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/java-bigquery API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bigquery: array-of-structs not working as named parameter in the java library
3 participants