Skip to content

fix: add support for repeated record query parameters #2698

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
May 25, 2023
Merged

fix: add support for repeated record query parameters #2698

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2c891e7
fix: add support for repeated record query parameters
obada-ab May 15, 2023
eefcdae
fix: set the query parameter type to an empty struct array if values …
obada-ab May 17, 2023
f8e36ce
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 17, 2023
8deb47a
Merge branch 'main' into struct-array
obada-ab May 17, 2023
f15630a
chore: add test for a real dataset
Neenu1995 May 18, 2023
6fc4085
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 18, 2023
0353c4e
chore: set up repeated record test to run in service account
obada-ab May 22, 2023
b5ac4af
doc: create and query repeated record sample
obada-ab May 25, 2023
49fa251
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 25, 2023
746db7a
refactor: reformat repeated record sample
obada-ab May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/QueryParameterValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import javax.annotation.Nullable;
import org.threeten.bp.Instant;
import org.threeten.bp.ZoneOffset;
Expand Down Expand Up @@ -349,7 +350,11 @@ public static <T> QueryParameterValue array(T[] array, Class<T> clazz) {
public static <T> QueryParameterValue array(T[] array, StandardSQLTypeName type) {
List<QueryParameterValue> listValues = new ArrayList<>();
for (T obj : array) {
listValues.add(QueryParameterValue.of(obj, type));
if (type == StandardSQLTypeName.STRUCT) {
listValues.add((QueryParameterValue) obj);
} else {
listValues.add(QueryParameterValue.of(obj, type));
}
}
return QueryParameterValue.newBuilder()
.setArrayValues(listValues)
Expand Down Expand Up @@ -522,9 +527,16 @@ QueryParameterType toTypePb() {
QueryParameterType typePb = new QueryParameterType();
typePb.setType(getType().toString());
if (getArrayType() != null) {
QueryParameterType arrayTypePb = new QueryParameterType();
arrayTypePb.setType(getArrayType().toString());
typePb.setArrayType(arrayTypePb);
if (getArrayType() == StandardSQLTypeName.STRUCT) {
List<QueryParameterValue> values =
Objects.requireNonNull(getArrayValues(), "Array of struct cannot be empty");
Copy link
Contributor

@Neenu1995 Neenu1995 May 15, 2023

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
Contributor Author

@obada-ab obada-ab May 15, 2023

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.

QueryParameterType structType = values.get(0).toTypePb();
typePb.setArrayType(structType);
} else {
QueryParameterType arrayTypePb = new QueryParameterType();
arrayTypePb.setType(getArrayType().toString());
typePb.setArrayType(arrayTypePb);
}
}
if (getStructTypes() != null) {
List<QueryParameterType.StructTypes> structTypes = new ArrayList<>();
Expand Down
43 changes: 43 additions & 0 deletions google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/QueryParameterValueTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.math.BigDecimal;
import java.text.ParseException;
import java.time.Period;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -563,6 +564,48 @@ public void testNestedStruct() {
assertThat(nestedRecordField.getStructValues().size()).isEqualTo(structValue.size());
}

@Test
public void testStructArray() {
Boolean[] boolValues = new Boolean[] {true, false};
Integer[] intValues = new Integer[] {15, 20};
String[] stringValues = new String[] {"test-string", "test-string2"};
List<ImmutableMap<String, QueryParameterValue>> fieldMaps = new ArrayList<>();
List<QueryParameterValue> tuples = new ArrayList<>();
for (int i = 0; i < 2; i++) {
QueryParameterValue booleanField = QueryParameterValue.bool(boolValues[i]);
QueryParameterValue integerField = QueryParameterValue.int64(intValues[i]);
QueryParameterValue stringField = QueryParameterValue.string(stringValues[i]);
ImmutableMap<String, QueryParameterValue> fieldMap =
ImmutableMap.of(
"booleanField",
booleanField,
"integerField",
integerField,
"stringField",
stringField);
fieldMaps.add(fieldMap);
QueryParameterValue recordField = QueryParameterValue.struct(fieldMap);
tuples.add(recordField);
}
QueryParameterValue repeatedRecordField =
QueryParameterValue.array(tuples.toArray(), StandardSQLTypeName.STRUCT);
com.google.api.services.bigquery.model.QueryParameterValue parameterValue =
repeatedRecordField.toValuePb();
QueryParameterType parameterType = repeatedRecordField.toTypePb();
QueryParameterValue queryParameterValue =
QueryParameterValue.fromPb(parameterValue, parameterType);
assertThat(queryParameterValue.getValue()).isNull();
assertThat(queryParameterValue.getType()).isEqualTo(StandardSQLTypeName.ARRAY);
assertThat(queryParameterValue.getArrayType()).isEqualTo(StandardSQLTypeName.STRUCT);
assertThat(queryParameterValue.getArrayValues().size()).isEqualTo(2);
for (int i = 0; i < 2; i++) {
QueryParameterValue record = queryParameterValue.getArrayValues().get(i);
assertThat(record.getType()).isEqualTo(StandardSQLTypeName.STRUCT);
assertThat(record.getStructTypes()).isNotNull();
assertThat(record.getStructValues()).isEqualTo(fieldMaps.get(i));
}
}

private static void assertArrayDataEquals(
String[] expectedValues,
StandardSQLTypeName expectedType,
Expand Down
41 changes: 41 additions & 0 deletions google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4062,6 +4062,47 @@ public void testStructNamedQueryParameters() throws InterruptedException {
}
}

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

@Neenu1995 Neenu1995 May 15, 2023

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
Contributor Author

@obada-ab obada-ab May 15, 2023

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

String[] stringValues = new String[] {"test-stringField", "test-stringField2"};
List<QueryParameterValue> tuples = new ArrayList<>();
for (int i = 0; i < 2; i++) {
QueryParameterValue stringValue = QueryParameterValue.string(stringValues[i]);
Map<String, QueryParameterValue> struct = new HashMap<>();
struct.put("stringField", stringValue);
QueryParameterValue recordValue = QueryParameterValue.struct(struct);
tuples.add(recordValue);
}

QueryParameterValue repeatedRecord =
QueryParameterValue.array(tuples.toArray(), StandardSQLTypeName.STRUCT);
String query = "SELECT @repeatedRecordField AS repeatedRecord";
QueryJobConfiguration config =
QueryJobConfiguration.newBuilder(query)
.setDefaultDataset(DATASET)
.setUseLegacySql(false)
.addNamedParameter("repeatedRecordField", repeatedRecord)
.build();
TableResult result = bigquery.query(config);
assertEquals(1, Iterables.size(result.getValues()));

FieldList subSchema = result.getSchema().getFields().get("repeatedRecord").getSubFields();
for (FieldValueList values : result.iterateAll()) {
for (FieldValue value : values) {
assertEquals(FieldValue.Attribute.REPEATED, value.getAttribute());
assertEquals(2, value.getRepeatedValue().size());
for (int i = 0; i < 2; i++) {
FieldValue record = value.getRepeatedValue().get(i);
assertEquals(FieldValue.Attribute.RECORD, record.getAttribute());
FieldValueList recordValue = record.getRecordValue();
assertEquals(
stringValues[i],
FieldValueList.of(recordValue, subSchema).get("stringField").getValue());
}
}
}
}

@Test
public void testStructQuery() throws InterruptedException {
// query into a table
Expand Down