reuvenlax commented on code in PR #27866:
URL: https://github.com/apache/beam/pull/27866#discussion_r1286593346
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOWriteTest.java:
##########
@@ -1090,13 +1094,15 @@ public void runTestWriteAvro(boolean schemaFromView)
throws Exception {
p.run();
+ System.err.println(
+ "ALLROWS " + fakeDatasetService.getAllRows("project-id", "dataset-id",
"table-id"));
Review Comment:
done
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProtoTest.java:
##########
@@ -566,48 +892,71 @@ public class TableRowToStorageApiProtoTest {
.add(
new TableFieldSchema()
.setType("STRUCT")
- .setName("nestedValue1")
+ .setName("nestedvalue1")
+ .setMode("NULLABLE")
.setFields(BASE_TABLE_SCHEMA.getFields()))
.add(
new TableFieldSchema()
.setType("RECORD")
- .setName("nestedValue2")
+ .setName("nestedvalue2")
+ .setMode("NULLABLE")
.setFields(BASE_TABLE_SCHEMA.getFields()))
.add(
new TableFieldSchema()
.setType("STRUCT")
- .setName("nestedValueNoF1")
+ .setName("nestedvaluenof1")
+ .setMode("NULLABLE")
.setFields(BASE_TABLE_SCHEMA_NO_F.getFields()))
.add(
new TableFieldSchema()
.setType("RECORD")
- .setName("nestedValueNoF2")
+ .setName("nestedvaluenof2")
+ .setMode("NULLABLE")
.setFields(BASE_TABLE_SCHEMA_NO_F.getFields()))
.build());
@Rule public transient ExpectedException thrown = ExpectedException.none();
@Test
- public void testDescriptorFromTableSchema() {
+ public void testDescriptorFromTableSchema() throws Exception {
DescriptorProto descriptor =
TableRowToStorageApiProto.descriptorSchemaFromTableSchema(BASE_TABLE_SCHEMA,
true, false);
Map<String, Type> types =
descriptor.getFieldList().stream()
.collect(
Collectors.toMap(FieldDescriptorProto::getName,
FieldDescriptorProto::getType));
Map<String, Type> expectedTypes =
- BASE_TABLE_SCHEMA_PROTO.getFieldList().stream()
+ BASE_TABLE_SCHEMA_PROTO_DESCRIPTOR.getFieldList().stream()
.collect(
Collectors.toMap(FieldDescriptorProto::getName,
FieldDescriptorProto::getType));
assertEquals(expectedTypes, types);
+
+ com.google.cloud.bigquery.storage.v1.TableSchema roundtripSchema =
+ TableRowToStorageApiProto.tableSchemaFromDescriptor(
+ TableRowToStorageApiProto.wrapDescriptorProto(descriptor));
+ Map<String, com.google.cloud.bigquery.storage.v1.TableFieldSchema.Type>
roundTripTypes =
+ roundtripSchema.getFieldsList().stream()
+ .collect(
+ Collectors.toMap(
+
com.google.cloud.bigquery.storage.v1.TableFieldSchema::getName,
+
com.google.cloud.bigquery.storage.v1.TableFieldSchema::getType));
+
+ Map<String, com.google.cloud.bigquery.storage.v1.TableFieldSchema.Type>
roundTripExpectedTypes =
+ BASE_TABLE_PROTO_SCHEMA.getFieldsList().stream()
+ .collect(
+ Collectors.toMap(
+
com.google.cloud.bigquery.storage.v1.TableFieldSchema::getName,
+
com.google.cloud.bigquery.storage.v1.TableFieldSchema::getType));
+
+ assertEquals(roundTripExpectedTypes, roundTripTypes);
Review Comment:
yeah, this is a weakness of this entire test. The earlier tests (converting
between json and proto) also don't test this, as we don't have any required
fields in the schema. I'll look into extending the test.
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProtoTest.java:
##########
@@ -63,111 +63,293 @@
/** Unit tests for {@link
org.apache.beam.sdk.io.gcp.bigquery.TableRowToStorageApiProto}. */
public class TableRowToStorageApiProtoTest {
// Schemas we test.
- // The TableRow class has special semantics for fields named "f". To ensure
we handel them
+ // The TableRow class has special semantics for fields named "f". To ensure
we handle them
// properly, we test schemas
// both with and without a field named "f".
private static final TableSchema BASE_TABLE_SCHEMA =
new TableSchema()
.setFields(
ImmutableList.<TableFieldSchema>builder()
- .add(new
TableFieldSchema().setType("STRING").setName("stringValue"))
- .add(new TableFieldSchema().setType("STRING").setName("f"))
- .add(new
TableFieldSchema().setType("BYTES").setName("bytesValue"))
- .add(new
TableFieldSchema().setType("INT64").setName("int64Value"))
- .add(new
TableFieldSchema().setType("INTEGER").setName("intValue"))
- .add(new
TableFieldSchema().setType("FLOAT64").setName("float64Value"))
- .add(new
TableFieldSchema().setType("FLOAT").setName("floatValue"))
- .add(new
TableFieldSchema().setType("BOOL").setName("boolValue"))
- .add(new
TableFieldSchema().setType("BOOLEAN").setName("booleanValue"))
- .add(new
TableFieldSchema().setType("TIMESTAMP").setName("timestampValue"))
- .add(new
TableFieldSchema().setType("TIME").setName("timeValue"))
- .add(new
TableFieldSchema().setType("DATETIME").setName("datetimeValue"))
- .add(new
TableFieldSchema().setType("DATE").setName("dateValue"))
- .add(new
TableFieldSchema().setType("NUMERIC").setName("numericValue"))
- .add(new
TableFieldSchema().setType("BIGNUMERIC").setName("bigNumericValue"))
- .add(new
TableFieldSchema().setType("NUMERIC").setName("numericValue2"))
- .add(new
TableFieldSchema().setType("BIGNUMERIC").setName("bigNumericValue2"))
+ .add(
Review Comment:
reverted all of these changes - no longer needed (they were part of an
earlier attempt to test)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
