TheNeuralBit commented on a change in pull request #14639:
URL: https://github.com/apache/beam/pull/14639#discussion_r621726193
##########
File path:
runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/util/SchemaCoderCloudObjectTranslator.java
##########
@@ -99,6 +99,9 @@ public SchemaCoder fromCloudObject(CloudObject cloudObject) {
SchemaApi.Schema.Builder schemaBuilder = SchemaApi.Schema.newBuilder();
JsonFormat.parser().merge(Structs.getString(cloudObject, SCHEMA),
schemaBuilder);
Schema schema = SchemaTranslation.schemaFromProto(schemaBuilder.build());
+ if (schema.isEncodingPositionsOverridden()) {
+ SchemaCoder.overrideEncodingPositions(schema.getUUID(),
schema.getEncodingPositions());
+ }
Review comment:
These calls aren't passing the null checker since `schema.getUUID()`
returns a nullable UUID
##########
File path: model/pipeline/src/main/proto/schema.proto
##########
@@ -37,6 +37,8 @@ message Schema {
// REQUIRED. An RFC 4122 UUID.
string id = 2;
repeated Option options = 3;
+ // Indicates that encoding positions have been overridden.
+ bool encoding_positions_set = 4;
Review comment:
Ack thank you.
nit: can you update the comment for encoding_position comment to reflect
this, something like:
```
// OPTIONAL. The position of this field's data when encoded, e.g. with
beam:coder:row:v1.
// Used to support backwards compatibility with schema changes.
// If this Field is part of a Schema where encoding_positions_set is True
then encoding_position must be
// defined, otherwise this field is ignored.
int32 encoding_position = 5;
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
