[FLINK-40492][table] Support casting from VARIANT to TIME and nanosecond timestamps - #29080
[FLINK-40492][table] Support casting from VARIANT to TIME and nanosecond timestamps#29080manner wants to merge 4 commits into
Conversation
| if (type != Variant.Type.TIMESTAMP && type != Variant.Type.TIMESTAMP_NS) { | ||
| throw unsupportedKind(variant, String.format("TIMESTAMP(%d)", precision)); | ||
| } | ||
| return DateTimeUtils.truncate( |
There was a problem hiding this comment.
There seems to be a pre-existing bug here, as DateTimeUtils.truncate uses Integer.toString(ts.toLocalDateTime().getNano()) to check the input's precision, which will drop the leading zeros.
This results in casting a TIMESTAMP_NS value with fraction .000123456 to TIMESTAMP(6) keeping all its nine digits, as Integer.toString() will return a string with a length of 6.
The following test would fail:
CastTestSpecBuilder.testCastTo(TIMESTAMP(6))
.fromCase(
VARIANT(),
Variant.newBuilder().of(LocalDateTime.of(2020, 1, 1, 12, 0, 0, 123_456)),
TimestampData.fromLocalDateTime(LocalDateTime.of(2020, 1, 1, 12, 0, 0, 123_000)))
with:
Expected :2020-01-01T12:00:00.000123
Actual :2020-01-01T12:00:00.000123456
Could you please fix the helper and include this test?
There was a problem hiding this comment.
Thanks for catching this @mateczagany! I'll add a fix in this PR!
There was a problem hiding this comment.
Are we planing to add more docs on the assumptions we take for the TIME precision cast?
There was a problem hiding this comment.
Yes, I was thinking we'll add more docs for the new Variant types (UUID, time and timestamps) in https://issues.apache.org/jira/browse/FLINK-40494
But I added a bit more now to this PR
| CastTestSpecBuilder.testCastTo(TIME(3)) | ||
| .fromCase( | ||
| VARIANT(), | ||
| Variant.newBuilder().of(LocalTime.of(12, 0, 0, 123_000_000)), |
There was a problem hiding this comment.
This is up to you but I kinda like this API a bit more:
| Variant.newBuilder().of(LocalTime.of(12, 0, 0, 123_000_000)), | |
| Variant.newBuilder().of(LocalTime.of(12, 0, 0).plus(Duration.ofMillis(123)), |
There was a problem hiding this comment.
I changed it for this occurrence only, as I think it would be harder to understand if the different APIs get mixed in a single test case. e.g.
.fromCase(
VARIANT(),
Variant.newBuilder().of(LocalTime.of(12, 0, 0, 123_456_000)),
DateTimeUtils.toInternal(LocalTime.of(12, 0, 0).plus(Duration.ofMillis(123)))
What is the purpose of the change
Extends VARIANT-to-scalar casting to the temporal types added in FLINK-40491:
TIMEand nanosecond-precisionTIMESTAMP/TIMESTAMP_LTZ.A VARIANT holding such a value can now be cast to
TIME(3),TIMESTAMP(9),TIMESTAMP_LTZ(9)and to a string, withCASTfailing /TRY_CASTreturningNULLon a kind mismatch.Brief change log
CAST/TRY_CASTfromVARIANTtoTIMETIMESTAMP_NS/TIMESTAMP_LTZ_NSstorage kinds when casting toTIMESTAMP(p)/TIMESTAMP_LTZ(p)TIME/TIMESTAMP_NS/TIMESTAMP_LTZ_NSwhen castingVARIANTto a stringVerifying this change
This change added tests in
CastRulesTest(VARIANT→TIME/TIMESTAMP(9)/TIMESTAMP_LTZ(9), both storage kinds, plus string rendering),LogicalTypeCastsTestandCastRuleProviderTest.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
VARIANT)Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Claude Opus 4.8)