twalthr commented on code in PR #29080:
URL: https://github.com/apache/flink/pull/29080#discussion_r3932863333
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java:
##########
@@ -1725,7 +1725,8 @@ private static void int4(StringBuilder buf, int i) {
}
public static TimestampData truncate(TimestampData ts, int precision) {
- String fraction = Integer.toString(ts.toLocalDateTime().getNano());
+ // Zero-pad to nine digits so leading zeros are not dropped and the
precision understated.
+ String fraction = String.format("%09d",
ts.toLocalDateTime().getNano());
Review Comment:
Translating over string is a pretty expensive operation. We should aim for
performance on the hotpath (per record evaluations). Can we rewrite this into a
more mathematical helper method?
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/VariantCastUtils.java:
##########
@@ -180,24 +184,42 @@ private static String decimalTarget(int precision, int
scale) {
}
/**
- * Reads a timestamp variant as the target {@code TIMESTAMP}. A variant
keeps microseconds, so
- * fractional seconds beyond the target precision are truncated, the same
as a regular {@code
- * TIMESTAMP} to {@code TIMESTAMP(p)} cast.
+ * Reads a timestamp variant as the target {@code TIMESTAMP}. {@link
Variant#getDateTime()}
+ * already accepts both the microsecond ({@link Variant.Type#TIMESTAMP})
and nanosecond ({@link
Review Comment:
```suggestion
* accepts both the microsecond ({@link Variant.Type#TIMESTAMP}) and
nanosecond ({@link
```
for the future: exclude historical context during AI generation
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
