from_unixtime might consider handle week year format

[shiyu-bytedance]

Description

Currently, DateTimeFormatter treats Y and y the same.

Strictly speaking, this is incorrect because Y should only be used for week year format. (see https://errorprone.info/bugpattern/MisusedWeekYear#:~:text=%E2%80%9CWeek%20year%E2%80%9D%20is%20intended%20to,year%20specifier%20%E2%80%9Cyyyy%E2%80%9D%20instead.)

Velox datetime functions currently do not support the week year style of formatting(https://en.wikipedia.org/wiki/ISO_8601#Week_dates), we might consider adding support for it.

Additional pointer:

• https://howardhinnant.github.io/date/iso_week.html already does the heavy lifting and we could leverage it to implement week year support.
• spark's from_unixtime has support for this format.

cc @pedroerp

[pedroerp]

@shiyu-bytedance seems like this is already correctly implemented in format_datetime()

presto> select format_datetime(cast('2014-12-30' as timestamp), 'x w y');
    _col0             
-------------
 2015 1 2014 

[shiyu-bytedance]

@pedroerp thanks. I'm surprised because I'd be not sure how this test passes.

velox/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp

Line 3251 in e13c591

formatDatetime(parseTimestamp("1970-01-01"), "x"), VeloxUserError);

[pedroerp]

I'm puzzled now as well; @amitkdutta any chance you know how this works in Prestissimo today?

[shiyu-bytedance]

@amitkdutta ping

[pedroerp]

@shiyu-bytedance ignore my comment above. You are right, it is not supported today. My test above ended up being executed in the Java stack :(

Would you like to add support for it?

[shiyu-bytedance]

@pedroerp Yes, I'd like to add support for it. However, to manage expectation, this issue, as we internally at Bytedance realized recently, is more involved than meets the eye.

Hinnant's library implements ISO8601 strictly; however, Presto/Spark uses SimpleDateFormatter(from .

These two libraries do not agree 100% on how to format certain edge cases; so for 100% compatibility, before diving straight into coding, I think it'd be advisable to think about how to tackle the issue of velox behaving identical to presto and spark (who are the main users of velox as an acceleration library), which may require net new code that bridges the gap instead of just invoking Hinnant's library.

[pedroerp]

Thanks for clarifying. If Presto is not following ISO8601 (which I suppose it should), maybe this is something we should bring up with the Presto community and get fixed? Have you tried opening an issue to prestodb?