You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/01/13 06:51:00 UTC

[jira] [Work logged] (AVRO-2123) Logical types timestamp-micros, duration, decimal bug with SpecificRecord

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

ASF GitHub Bot logged work on AVRO-2123:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 13/Jan/22 06:50
            Start Date: 13/Jan/22 06:50
    Worklog Time Spent: 10m 
      Work Description: opwvhk commented on a change in pull request #1263:
URL: https://github.com/apache/avro/pull/1263#discussion_r783670909



##########
File path: lang/java/avro/src/main/java/org/apache/avro/Conversions.java
##########
@@ -146,6 +149,111 @@ private static BigDecimal validate(final LogicalTypes.Decimal decimal, BigDecima
     }
   }
 
+  public static class DurationConversion extends Conversion<Duration> {
+
+      private static final int MONTH_DAYS = 30;

Review comment:
       A month has 28, 29, 30 or 31 days. Hardcoding a single value (31 days/month occurs more often) is basically a bug. Note that this cannot be fixed if we choose to support all possible duration values (see the conversation comments for more info).




-- 
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: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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

            Worklog Id:     (was: 708090)
    Remaining Estimate: 2h 40m  (was: 2h 50m)
            Time Spent: 20m  (was: 10m)

> Logical types timestamp-micros, duration, decimal bug with SpecificRecord
> -------------------------------------------------------------------------
>
>                 Key: AVRO-2123
>                 URL: https://issues.apache.org/jira/browse/AVRO-2123
>             Project: Apache Avro
>          Issue Type: Bug
>          Components: java
>    Affects Versions: 1.8.2
>         Environment: All (Linux and Windows)
>            Reporter: Daniel Egloff
>            Priority: Critical
>              Labels: pull-request-available
>         Attachments: FixedDuration.java, TestRecordWithLogicalTypesBroken.java, specific_types_broken.avsc
>
>   Original Estimate: 3h
>          Time Spent: 20m
>  Remaining Estimate: 2h 40m
>
> The logical types date, time-millis, timestamp-millis are all handled properly with a converter. Specifying a schema and generate code results in a good constructor, which an use the proper types and serializatio e.g. from and to Json works as expected. 
> However if  timestamp-micros, duration, decimal are used in the schema, there is no converter being generated.
> Consider this schema
> {
>   "type" : "record",
>   "name" : "TestRecordWithLogicalTypesBroken",
>   "doc" : "Schema from Avro Project - the three last fields are not properly converted",
>   "namespace" : "com.flink.ai.kafka.lab.types",
>   "fields" : [ {
>     "name" : "b",
>     "type" : "boolean"
>   }, {
>     "name" : "i32",
>     "type" : "int"
>   }, {
>     "name" : "i64",
>     "type" : "long"
>   }, {
>     "name" : "f32",
>     "type" : "float"
>   }, {
>     "name" : "f64",
>     "type" : "double"
>   }, {
>     "name" : "s",
>     "type" : [ "null", "string" ],
>     "default" : null
>   }, {
>     "name" : "d",
>     "type" : {
>       "type" : "int",
>       "logicalType" : "date"
>     }
>   }, {
>     "name" : "t",
>     "type" : {
>       "type" : "int",
>       "logicalType" : "time-millis"
>     }
>   }, {
>     "name" : "ts",
>     "type" : {
>       "type" : "long",
>       "logicalType" : "timestamp-millis"
>     }
>   }, {
>     "name" : "tsm",
>     "type" : {
>       "type" : "long",
>       "logicalType" : "timestamp-micros"
>     }
>   }, {
>     "name" : "dur",
>     "type" : {
>       "type" : "fixed",
>       "size" : 12,
>       "name" : "FixedDuration",
>       "logicalType" : "duration"
>     }
>   }, {
>     "name" : "dec",
>     "type" : {
>       "type" : "bytes",
>       "logicalType" : "decimal",
>       "precision" : 9,
>       "scale" : 2
>     }
>   }]
> }
> It results in the following generated code: First the conversions:
> private static final org.apache.avro.Conversion<?>[] conversions =
>       new org.apache.avro.Conversion<?>[] {
>       null,
>       null,
>       null,
>       null,
>       null,
>       null,
>       DATE_CONVERSION,
>       TIME_CONVERSION,
>       TIMESTAMP_CONVERSION,
>       null,    // should not be  null, should be a proper timestamp converter
>       null,    // should not be null, should be a proper duration converter
>       null,    // should not be null, should be a proper Decimal converter
>       null
>   };
> Then also the constructor uses the underlying basic types.
> public TestRecordWithLogicalTypesBroken(java.lang.Boolean b, java.lang.Integer i32, java.lang.Long i64, java.lang.Float f32, java.lang.Double f64, java.lang.CharSequence s, org.joda.time.LocalDate d, org.joda.time.LocalTime t, org.joda.time.DateTime ts, java.lang.Long tsm, com.flink.ai.kafka.lab.types.FixedDuration dur, java.nio.ByteBuffer dec) {...}
>  
> This is not in line with the documentation here:
> https://avro.apache.org/docs/1.8.2/spec.html#Timestamp+%28microsecond+precision%29
> https://avro.apache.org/docs/1.8.2/spec.html#Duration
> https://avro.apache.org/docs/1.8.2/spec.html#Decimal
> Do I somehow use the code generation wrong?
> I also observed that in the test there is a frozen class 
> https://github.com/apache/avro/blob/master/lang/java/avro/src/test/java/org/apache/avro/specific/TestRecordWithLogicalTypes.java
> If I use the code generation with 1.8.2 I cannot reproduce this. Also note that it has a Decimal converter generated:
>   private final org.apache.avro.Conversion<?>[] conversions =
>       new org.apache.avro.Conversion<?>[] {
>       null,
>       null,
>       null,
>       null,
>       null,
>       null,
>       DATE_CONVERSION,
>       TIME_CONVERSION,
>       TIMESTAMP_CONVERSION,
>       DECIMAL_CONVERSION,
>       null
>   };
> I attach the schema that I used and the code that is generated from it.
> Thanks for looking into this.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)