CharSyam, As I understood, it seems like Date#fromLogical assumes that the given Data object is created under UTC timezone - its unit test, DateTest, creates all related GregorianCalendar objects with TimeZone.getTimeZone("UTC"). So if all the Date objects given as the parameter are UTC-oriented, it seems like this implementation makes any problem. So: Have ever experienced the case of non-UTC Date object? I inspected all classes implementing Transformation interface[^1] but all the classes related to Date or Time[^2] were using UTC-oriented Date object. (check the `TimeZone.getTimeZone("UTC")` clause from the code.) If there is a logical crack, it will worth a fix.
Thanks, Dongjin [^1]: https://cwiki.apache.org/confluence/display/KAFKA/KIP-66:+Single+Message+Transforms+for+Kafka+Connect [^2]: TimestampRouter, TimestampConverter. On Mon, Oct 30, 2017 at 4:45 PM, CharSyam <chars...@gmail.com> wrote: > Hi here. > > I have a question about fromLogical method in > connect/api/src/main/java/org/apache/kafka/connect/data/Date.java > > below code does these step. > 1. calendar with UTC timezone > 2. set value > 3. check hour, minute, second, millisecond are 0. if not throw > DataException. > > ``` > public static int fromLogical(Schema schema, java.util.Date value) { > if (schema.name() == null || !(schema.name().equals(LOGICAL_NAME))) > throw new DataException("Requested conversion of Date object > but the schema does not match."); > Calendar calendar = Calendar.getInstance(UTC); > calendar.setTime(value); > if (calendar.get(Calendar.HOUR_OF_DAY) != 0 || > calendar.get(Calendar.MINUTE) != 0 || > calendar.get(Calendar.SECOND) != 0 || > calendar.get(Calendar.MILLISECOND) != 0) { > throw new DataException("Kafka Connect Date type should not > have any time fields set to non-zero values."); > } > long unixMillis = calendar.getTimeInMillis(); > return (int) (unixMillis / MILLIS_PER_DAY); > } > ``` > > but, if value is not made by UTC, for if we read value from other > Timezone, the Date can be represented other value, because timezone > will make difference. > > so I think this code cause failure when different timezone. > > I think revising with timezone info is a way to solve this. > ``` > public static int fromLogical(Schema schema, java.util.Date value) { > if (schema.name() == null || !(schema.name().equals(LOGICAL_NAME))) > throw new DataException("Requested conversion of Date object > but the schema does not match."); > //Using DefaultTimeZone > Calendar calendar = Calendar.getInstance(TimeZone.getDefault()); > calendar.setTime(value); > if (calendar.get(Calendar.HOUR_OF_DAY) != 0 || > calendar.get(Calendar.MINUTE) != 0 || > calendar.get(Calendar.SECOND) != 0 || > calendar.get(Calendar.MILLISECOND) != 0) { > throw new DataException("Kafka Connect Date type should not > have any time fields set to non-zero values."); > } > > //Calculate timezone difference as reviseMillis > long reviseMillis = calendar.get(Calendar.ZONE_OFFSET) + > calendar.get(Calendar.DST_OFFSET) / 60; > > //revising with reviseMillis > long unixMillis = calendar.getTimeInMillis() + reviseMillis; > return (int) (unixMillis / MILLIS_PER_DAY); > } > > public static java.util.Date toLogical(Schema schema, int value) { > if (schema.name() == null || !(schema.name().equals(LOGICAL_NAME))) > throw new DataException("Requested conversion of Date object > but the schema does not match."); > > //Calculate timezone difference as reviseMillis > Calendar calendar = Calendar.getInstance(TimeZone.getDefault()); > long reviseMillis = calendar.get(Calendar.ZONE_OFFSET) + > calendar.get(Calendar.DST_OFFSET) / 60; > //revising with reviseMillis > return new java.util.Date(value * MILLIS_PER_DAY - reviseMillis); > } > ``` > > But I wonder, is there some reason to force this work? > to check not to have any time fields set to non-zero values? > > What do you think guys? > -- *Dongjin Lee* *A hitchhiker in the mathematical world.* *github: <http://goog_969573159/>github.com/dongjinleekr <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr <http://kr.linkedin.com/in/dongjinleekr>slideshare: www.slideshare.net/dongjinleekr <http://www.slideshare.net/dongjinleekr>*