You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by rs...@apache.org on 2021/12/11 14:33:05 UTC

[avro] branch branch-1.11 updated: AVRO-2976: Add LogicalType parsing to the IDL (#985)

This is an automated email from the ASF dual-hosted git repository.

rskraba pushed a commit to branch branch-1.11
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/branch-1.11 by this push:
     new becde91  AVRO-2976: Add LogicalType parsing to the IDL (#985)
becde91 is described below

commit becde91a74fc720443e1dfa5ed560c336271d47e
Author: Oscar Westra van Holthe - Kind <op...@users.noreply.github.com>
AuthorDate: Sat Dec 11 15:32:23 2021 +0100

    AVRO-2976: Add LogicalType parsing to the IDL (#985)
    
    * AVRO-2976: Add LogicalType parsing to IDL
    
    * AVRO-2976: Handle both Long and Integer in IDL JSON parsing
    
    The IDL parser now distinguishes numbers between Long and Integer. This
    is needed because the logical type decimal breaks on Long numbers for
    precision and scale.
    
    I chose to solve it here because the scope of this change is smaller. An
    alternative is to downcast a Long in the method
    LogicalTypes.Decimal#getInt(Schema, String).
    
    * AVRO-2976: Remove duplicate annotation processing for types.
    
    * Extend simple test to also verify annotations on arrays (the grammar alrewady supports this).
    
    * AVRO-2976: Trigger tests (build fail on unrelated part)
    
    * AVRO-2976: Revert pom.xml change.
    
    * AVRO-2976: Fix copy/paste error
    
    * AVRO-2976: Remove debugging code
    
    The removed code was used to evaluate if the test would work.
    
    * AVRO-2976: Spotless
    
    * AVRO-2976: Fix test
    
    The test worked because calling toString() on the protocol made schemas
    available by their unqualified name (without namespace).
---
 .../javacc/org/apache/avro/compiler/idl/idl.jj     | 18 ++++-
 lang/java/compiler/src/test/idl/input/simple.avdl  |  1 +
 lang/java/compiler/src/test/idl/logicalTypes.avdl  | 32 ++++++++
 lang/java/compiler/src/test/idl/output/simple.avpr |  4 +
 .../apache/avro/compiler/idl/TestLogicalTypes.java | 93 ++++++++++++++++++++++
 5 files changed, 145 insertions(+), 3 deletions(-)

diff --git a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
index d4be8a5..e6977cb 100644
--- a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
+++ b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
@@ -71,6 +71,7 @@ import java.util.Map;
 import java.net.URL;
 
 import org.apache.avro.Schema;
+import org.apache.avro.LogicalType;
 import org.apache.avro.LogicalTypes;
 import org.apache.avro.Schema.*;
 import org.apache.avro.Protocol;
@@ -1058,6 +1059,9 @@ private Schema NamedSchemaDeclaration(Map<String, JsonNode> props):
      } else {                                     // add all other props
        Accessor.addProp(s, key, props.get(key));
      }
+   LogicalType logicalType = LogicalTypes.fromSchemaIgnoreInvalid(s);
+   if (logicalType != null)
+     logicalType.addToSchema(s);
 
    return s;
  }
@@ -1314,7 +1318,6 @@ private void SchemaProperty(Map<String, JsonNode> properties):
 void FieldDeclaration(List<Field> fields):
 {
   Schema type;
-  Map<String, JsonNode> props = new LinkedHashMap<String, JsonNode>();
 }
 {
   type = Type()
@@ -1430,12 +1433,14 @@ Schema Type():
   Map<String, JsonNode> props = new LinkedHashMap<String, JsonNode>();
 }
 {
-
     ( SchemaProperty(props) )*
   s = UnannotatedType(props)
   {
     for (String key : props.keySet())
       Accessor.addProp(s, key, props.get(key));
+    LogicalType logicalType = LogicalTypes.fromSchemaIgnoreInvalid(s);
+    if (logicalType != null)
+      logicalType.addToSchema(s);
     return s;
   }
 }
@@ -1617,7 +1622,14 @@ private JsonNode Json() :
 { String s; Token t; JsonNode n; }
 { 
 ( s = JsonString() { n = new TextNode(s); }
-| (t=<INTEGER_LITERAL> { n = new LongNode(Long.parseLong(t.image)); })
+| (t=<INTEGER_LITERAL> {
+  long longValue = Long.parseLong(t.image);
+  int intValue = (int)longValue;
+  if (intValue == longValue)
+    n = new IntNode(intValue);
+  else
+    n = new LongNode(longValue);
+})
 | (t=<FLOATING_POINT_LITERAL> {n=new DoubleNode(Double.parseDouble(t.image));})
 | n=JsonObject()
 | n=JsonArray()
diff --git a/lang/java/compiler/src/test/idl/input/simple.avdl b/lang/java/compiler/src/test/idl/input/simple.avdl
index a9f67ed..c0df9f3 100644
--- a/lang/java/compiler/src/test/idl/input/simple.avdl
+++ b/lang/java/compiler/src/test/idl/input/simple.avdl
@@ -60,6 +60,7 @@ protocol Simple {
     time_ms t = 0;
 
     @foo.bar("bar.foo") long l = 0;
+    @foo.bar.bar("foo.bar2") array<string> a = [];
     union {null, @foo.foo.bar(42) @foo.foo.foo("3foo") string} prop = null;
   }
 
diff --git a/lang/java/compiler/src/test/idl/logicalTypes.avdl b/lang/java/compiler/src/test/idl/logicalTypes.avdl
new file mode 100644
index 0000000..9e4a284
--- /dev/null
+++ b/lang/java/compiler/src/test/idl/logicalTypes.avdl
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+@version("1.0.5")
+@namespace("org.apache.avro.test")
+protocol LogicalTypeTest {
+    record LogicalTypeFields {
+        date aDate;
+        time_ms aTime;
+        timestamp_ms aTimestamp;
+        local_timestamp_ms aLocalTimestamp;
+        decimal(6,2) pocketMoney;
+        uuid identifier;
+        @logicalType("timestamp-micros") long anotherTimestamp;
+        @logicalType("decimal") @precision(6) @scale(2) bytes allowance;
+        @logicalType("decimal") @precision(3000000000) @scale(0) bytes byteArray;
+    }
+}
diff --git a/lang/java/compiler/src/test/idl/output/simple.avpr b/lang/java/compiler/src/test/idl/output/simple.avpr
index 6128d44..ab1c03c 100644
--- a/lang/java/compiler/src/test/idl/output/simple.avpr
+++ b/lang/java/compiler/src/test/idl/output/simple.avpr
@@ -71,6 +71,10 @@
       "type": {"type": "long", "foo.bar": "bar.foo"},
       "default": 0
     } , {
+      "name": "a",
+      "type": {"type": "array", "items": "string", "foo.bar.bar": "foo.bar2"},
+      "default": []
+    } , {
       "name": "prop",
       "type": [ "null" , {"type":"string", "foo.foo.bar": 42, "foo.foo.foo": "3foo"} ],
       "default": null
diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestLogicalTypes.java b/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestLogicalTypes.java
new file mode 100644
index 0000000..77cda5c
--- /dev/null
+++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestLogicalTypes.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.compiler.idl;
+
+import org.apache.avro.LogicalType;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.Protocol;
+import org.apache.avro.Schema;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestLogicalTypes {
+  private Schema logicalTypeFields;
+
+  @Before
+  public void setup() throws ParseException {
+    final ClassLoader cl = Thread.currentThread().getContextClassLoader();
+    Idl idl = new Idl(cl.getResourceAsStream("logicalTypes.avdl"), "UTF-8");
+    Protocol protocol = idl.CompilationUnit();
+
+    logicalTypeFields = protocol.getType("org.apache.avro.test.LogicalTypeFields");
+  }
+
+  @Test
+  public void testDateBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.date(), logicalTypeOfField("aDate"));
+  }
+
+  @Test
+  public void testTimeMsBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.timeMillis(), logicalTypeOfField("aTime"));
+  }
+
+  @Test
+  public void testTimestampMsBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.timestampMillis(), logicalTypeOfField("aTimestamp"));
+  }
+
+  @Test
+  public void testLocalTimestampMsBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.localTimestampMillis(), logicalTypeOfField("aLocalTimestamp"));
+  }
+
+  @Test
+  public void testDecimalBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.decimal(6, 2), logicalTypeOfField("pocketMoney"));
+  }
+
+  @Test
+  public void testUuidBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.uuid(), logicalTypeOfField("identifier"));
+  }
+
+  @Test
+  public void testAnnotatedLongBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.timestampMicros(), logicalTypeOfField("anotherTimestamp"));
+  }
+
+  @Test
+  public void testAnnotatedBytesFieldBecomesLogicalType() {
+    Assert.assertEquals(LogicalTypes.decimal(6, 2), logicalTypeOfField("allowance"));
+  }
+
+  @Test
+  public void testIncorrectlyAnnotatedBytesFieldHasNoLogicalType() {
+    Schema fieldSchema = logicalTypeFields.getField("byteArray").schema();
+
+    Assert.assertNull(fieldSchema.getLogicalType());
+    Assert.assertEquals("decimal", fieldSchema.getObjectProp("logicalType"));
+    Assert.assertEquals(3000000000L, fieldSchema.getObjectProp("precision")); // Not an int, so not a valid precision
+    Assert.assertEquals(0, fieldSchema.getObjectProp("scale"));
+  }
+
+  private LogicalType logicalTypeOfField(String name) {
+    return logicalTypeFields.getField(name).schema().getLogicalType();
+  }
+}