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>*

Reply via email to