[jira] [Work logged] (BEAM-9416) BIP-1: Convert avro metadata to Schema options

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/BEAM-9416?focusedWorklogId=479690&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-479690 ]

ASF GitHub Bot logged work on BEAM-9416:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 07/Sep/20 16:17
            Start Date: 07/Sep/20 16:17
    Worklog Time Spent: 10m 
      Work Description: iemejia commented on a change in pull request #11217:
URL: https://github.com/apache/beam/pull/11217#discussion_r484493417



##########
File path: sdks/java/core/src/test/java/org/apache/beam/sdk/schemas/utils/AvroUtilsTest.java
##########
@@ -260,8 +260,25 @@ private Schema getBeamSchema() {
         .addField(Field.of("double", FieldType.DOUBLE))
         .addField(Field.of("string", FieldType.STRING))
         .addField(Field.of("bytes", FieldType.BYTES))
-        .addField(Field.of("decimal", FieldType.DECIMAL))
-        .addField(Field.of("timestampMillis", FieldType.DATETIME))
+        .addField(

Review comment:
       If I understood correctly there are options at the Schema level, can we also set some options at the Schema level to test that conversion too.

##########
File path: build.gradle
##########
@@ -63,8 +63,7 @@ rat {
     "**/.github/**/*",
 
     "**/package-list",
-    "**/test.avsc",
-    "**/user.avsc",
+    "**/*.avsc",

Review comment:
       :+1: 

##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroUtils.java
##########
@@ -316,6 +444,15 @@ public static Schema toBeamSchema(org.apache.avro.Schema schema) {
       builder.addField(beamField);
     }
 
+    return builder.setOptions(toBeamSchemaOptions(schema)).build();
+  }
+
+  private static Schema.Options toBeamSchemaOptions(org.apache.avro.Schema schema) {
+    Schema.Options.Builder builder = Schema.Options.builder();
+    schema
+        .getJsonProps()

Review comment:
       Worth to replace with `.getObjectProps()` too

##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroUtils.java
##########
@@ -287,7 +297,125 @@ protected StackManipulation convertDefault(TypeDescriptor<?> type) {
   public static Schema.Field toBeamField(org.apache.avro.Schema.Field field) {
     TypeWithNullability nullableType = new TypeWithNullability(field.schema());
     FieldType beamFieldType = toFieldType(nullableType);
-    return Field.of(field.name(), beamFieldType);
+    return Field.of(field.name(), beamFieldType).withOptions(toBeamFieldOptions(field));
+  }
+
+  private static Schema.Options toBeamFieldOptions(org.apache.avro.Schema.Field field) {
+    Schema.Options.Builder builder = Schema.Options.builder();
+    field
+        .getJsonProps()

Review comment:
       `getJsonProps()` was removed on Avro >= 1.9 so we should probably do all this logic with `getObjectProps` instead. I know this can change a good chunk of the main code, but probably better to be forwards compatible.

##########
File path: sdks/java/core/src/test/java/org/apache/beam/sdk/schemas/AvroSchemaTest.java
##########
@@ -375,6 +464,12 @@ public void testSpecificRecordSchema() {
     assertEquals(SCHEMA, new AvroRecordSchema().schemaFor(TypeDescriptor.of(TestAvro.class)));
   }
 
+  @Test
+  public void testOptionsRecordSchema() {
+    assertEquals(
+        OPTIONS_SCHEMA, new AvroRecordSchema().schemaFor(TypeDescriptor.of(TestOptionsAvro.class)));

Review comment:
       :+1: good approach this test I like using a generated class for completeness.




----------------------------------------------------------------
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:
users@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 479690)
    Time Spent: 3h  (was: 2h 50m)

> BIP-1: Convert avro metadata to Schema options
> ----------------------------------------------
>
>                 Key: BEAM-9416
>                 URL: https://issues.apache.org/jira/browse/BEAM-9416
>             Project: Beam
>          Issue Type: Sub-task
>          Components: sdk-java-core
>            Reporter: Alex Van Boxel
>            Assignee: Alex Van Boxel
>            Priority: P2
>             Fix For: 2.22.0
>
>          Time Spent: 3h
>  Remaining Estimate: 0h
>
> Avro has some metadata that can be added to the normal type information. It is based on json typing, so the conversion will be best effort (probably we can bet int, string and float out of it).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)