matriv commented on a change in pull request #18632:
URL: https://github.com/apache/flink/pull/18632#discussion_r801422436
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java
##########
@@ -419,43 +401,32 @@ private static long toMillis(DecimalData v) {
// Parsing functions
//
--------------------------------------------------------------------------------------------
- public static TimestampData parseTimestampData(String dateStr) {
- int length = dateStr.length();
- String format;
- if (length == 10) {
- format = DATE_FORMAT_STRING;
- } else if (length >= 21 && length <= 29) {
- format = DEFAULT_DATETIME_FORMATS[length - 20];
- } else {
- // otherwise fall back to second's precision
- format = DEFAULT_DATETIME_FORMATS[0];
- }
- return parseTimestampData(dateStr, format);
+ public static TimestampData parseTimestampData(String dateStr) throws
DateTimeException {
+ // Precision is hardcoded to match signature of TO_TIMESTAMP
Review comment:
Maybe add a link to https://issues.apache.org/jira/browse/FLINK-14925
##########
File path:
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/data/binary/BinaryStringDataUtil.java
##########
@@ -604,19 +603,16 @@ public static int toTime(BinaryStringData input) throws
DateTimeException {
return date;
}
- public static TimestampData toTimestamp(BinaryStringData input) throws
DateTimeException {
- TimestampData timestamp =
DateTimeUtils.parseTimestampData(input.toString());
- if (timestamp == null) {
- throw new DateTimeException("For input string: '" + input + "'.");
- }
-
- return timestamp;
+ /** Used by {@code CAST(x as TIMESTAMP)}. */
+ public static TimestampData toTimestamp(BinaryStringData input, int
precision)
+ throws DateTimeException {
+ return DateTimeUtils.parseTimestampData(input.toString(), precision);
}
- public static TimestampData toTimestamp(BinaryStringData input, TimeZone
timeZone)
- throws ParseException {
- long timestamp = DateTimeUtils.parseTimestampMillis(input.toString(),
timeZone);
- return TimestampData.fromEpochMillis(timestamp);
+ /** Used by {@code CAST(x as TIMESTAMP_LTZ)}. */
+ public static TimestampData toTimestamp(
+ BinaryStringData input, TimeZone timeZone, int precision) throws
DateTimeException {
Review comment:
I would move `timeZone` as last argument (extra from the other
signature).
##########
File path:
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/StringToTimestampCastRule.java
##########
@@ -48,12 +51,24 @@ public String generateExpression(
String inputTerm,
LogicalType inputLogicalType,
LogicalType targetLogicalType) {
+ int targetPrecision =
LogicalTypeChecks.getPrecision(targetLogicalType);
if (targetLogicalType.is(LogicalTypeRoot.TIMESTAMP_WITHOUT_TIME_ZONE))
{
- return staticCall(STRING_DATA_TO_TIMESTAMP(), inputTerm);
+ final TimestampKind targetTimestampKind = ((TimestampType)
targetLogicalType).getKind();
+ if (targetTimestampKind == TimestampKind.ROWTIME
Review comment:
Maybe this is not needed at all, I don't think we can CAST to a
`ROWTIME/PROCTIME`, this can only be already set on the input field of a cast,
but the target is just a type, declared as a literal by the user. @twalthr
right?
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java
##########
@@ -477,28 +448,32 @@ public static TimestampData parseTimestampData(String
dateStr, String format) {
}
/**
- * Parse date time string to timestamp based on the given time zone and
"yyyy-MM-dd HH:mm:ss"
- * format. Returns null if parsing failed.
- *
- * @param dateStr the date time string
- * @param tz the time zone
+ * This is similar to {@link LocalDateTime#from(TemporalAccessor)}, but
it's less strict and
+ * introduces default values.
*/
- public static long parseTimestampMillis(String dateStr, TimeZone tz)
throws ParseException {
- int length = dateStr.length();
- String format;
- if (length == 10) {
- format = DATE_FORMAT_STRING;
- } else if (length == 21) {
- format = DEFAULT_DATETIME_FORMATS[1];
- } else if (length == 22) {
- format = DEFAULT_DATETIME_FORMATS[2];
- } else if (length == 23) {
- format = DEFAULT_DATETIME_FORMATS[3];
- } else {
- // otherwise fall back to the default
- format = DEFAULT_DATETIME_FORMATS[0];
- }
- return parseTimestampMillis(dateStr, format, tz);
+ private static LocalDateTime fromTemporalAccessor(TemporalAccessor
accessor, int precision) {
+ // complement year with 1970
+ int year = accessor.isSupported(YEAR) ? accessor.get(YEAR) : 1970;
+ // complement month with 1
+ int month = accessor.isSupported(MONTH_OF_YEAR) ?
accessor.get(MONTH_OF_YEAR) : 1;
+ // complement day with 1
+ int day = accessor.isSupported(DAY_OF_MONTH) ?
accessor.get(DAY_OF_MONTH) : 1;
+ // complement hour with 0
+ int hour = accessor.isSupported(HOUR_OF_DAY) ?
accessor.get(HOUR_OF_DAY) : 0;
+ // complement minute with 0
+ int minute = accessor.isSupported(MINUTE_OF_HOUR) ?
accessor.get(MINUTE_OF_HOUR) : 0;
+ // complement second with 0
+ int second = accessor.isSupported(SECOND_OF_MINUTE) ?
accessor.get(SECOND_OF_MINUTE) : 0;
+ // complement nano_of_second with 0
+ int nanoOfSecond = accessor.isSupported(NANO_OF_SECOND) ?
accessor.get(NANO_OF_SECOND) : 0;
+
+ if (precision == 0) {
+ nanoOfSecond = 0;
+ } else if (precision != 9) {
+ nanoOfSecond = (int) floor(nanoOfSecond, powerX(10, 9 -
precision));
Review comment:
We can have a static lookup map to get the i.e. "00" string with key the
`9-precision`.
##########
File path:
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/DateTimeUtils.java
##########
@@ -477,28 +448,32 @@ public static TimestampData parseTimestampData(String
dateStr, String format) {
}
/**
- * Parse date time string to timestamp based on the given time zone and
"yyyy-MM-dd HH:mm:ss"
- * format. Returns null if parsing failed.
- *
- * @param dateStr the date time string
- * @param tz the time zone
+ * This is similar to {@link LocalDateTime#from(TemporalAccessor)}, but
it's less strict and
+ * introduces default values.
*/
- public static long parseTimestampMillis(String dateStr, TimeZone tz)
throws ParseException {
- int length = dateStr.length();
- String format;
- if (length == 10) {
- format = DATE_FORMAT_STRING;
- } else if (length == 21) {
- format = DEFAULT_DATETIME_FORMATS[1];
- } else if (length == 22) {
- format = DEFAULT_DATETIME_FORMATS[2];
- } else if (length == 23) {
- format = DEFAULT_DATETIME_FORMATS[3];
- } else {
- // otherwise fall back to the default
- format = DEFAULT_DATETIME_FORMATS[0];
- }
- return parseTimestampMillis(dateStr, format, tz);
+ private static LocalDateTime fromTemporalAccessor(TemporalAccessor
accessor, int precision) {
+ // complement year with 1970
+ int year = accessor.isSupported(YEAR) ? accessor.get(YEAR) : 1970;
+ // complement month with 1
+ int month = accessor.isSupported(MONTH_OF_YEAR) ?
accessor.get(MONTH_OF_YEAR) : 1;
+ // complement day with 1
+ int day = accessor.isSupported(DAY_OF_MONTH) ?
accessor.get(DAY_OF_MONTH) : 1;
+ // complement hour with 0
+ int hour = accessor.isSupported(HOUR_OF_DAY) ?
accessor.get(HOUR_OF_DAY) : 0;
+ // complement minute with 0
+ int minute = accessor.isSupported(MINUTE_OF_HOUR) ?
accessor.get(MINUTE_OF_HOUR) : 0;
+ // complement second with 0
+ int second = accessor.isSupported(SECOND_OF_MINUTE) ?
accessor.get(SECOND_OF_MINUTE) : 0;
+ // complement nano_of_second with 0
+ int nanoOfSecond = accessor.isSupported(NANO_OF_SECOND) ?
accessor.get(NANO_OF_SECOND) : 0;
+
+ if (precision == 0) {
+ nanoOfSecond = 0;
+ } else if (precision != 9) {
+ nanoOfSecond = (int) floor(nanoOfSecond, powerX(10, 9 -
precision));
Review comment:
Would you mind checking with a jmh, if
`Integer.parseInt(String.valueOf(nanoOfSecond).substring(0, precision) + <<"0"
* 9 - precision)>>)` is better?
--
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]
