[
https://issues.apache.org/jira/browse/CALCITE-7560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18085876#comment-18085876
]
Mihai Budiu commented on CALCITE-7560:
--------------------------------------
I suspect this can be fixed in a similar way to CALCITE-7559
I assume you plan to submit a PR
> SqlFunctions.DateParseFunction.parseTimestamp(..., timeZone) accepts unknown
> time zones and silently falls back to GMT
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-7560
> URL: https://issues.apache.org/jira/browse/CALCITE-7560
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.41.0
> Reporter: Ruiqi Dong
> Priority: Major
>
> *Summary*
> Calcite's runtime implementation of BigQuery-style PARSE_TIMESTAMP(format,
> string [, timezone]) accepts unknown time-zone identifiers and interprets
> them as GMT. This is not just parser behavior. It affects runtime evaluation
> in SqlFunctions.DateParseFunction, so a typo in the time-zone argument
> changes query results instead of raising an error.
>
> *Affected code*
> Files:
> core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
> core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
> {code:java}
> /**
> * The "PARSE_TIMESTAMP(string, string [, timezone])" function (BigQuery);
> * Formats a timestamp object according to the specified string.
> */
> public static final SqlFunction PARSE_TIMESTAMP = ... {code}
> The runtime implementation uses TimeZone.getTimeZone(...) directly:
> {code:java}
> public long parseTimestamp(String fmtString, String timestamp,
> String timeZone) {
> TimeZone tz = TimeZone.getTimeZone(timeZone);
> final long millisSinceEpoch =
> internalParseDatetime(fmtString, timestamp, timeZone);
> return toLong(new java.sql.Timestamp(millisSinceEpoch), tz);
> } {code}
> The cached DateFormat path does the same:
> {code:java}
> TimeZone tz = TimeZone.getTimeZone(timeZone);
> parser.setCalendar(Calendar.getInstance(tz, Locale.ROOT)); {code}
> Because TimeZone.getTimeZone(...) silently falls back to GMT for unknown IDs,
> the user-supplied bad zone is never rejected.
>
> *Reproducer*
> Add the following test to
> core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java
> {code:java}
> @Test void testParseTimestampUnknownTimeZoneMatchesGmtFallback() {
> final SqlFunctions.DateParseFunction parse = new
> SqlFunctions.DateParseFunction();
> final long gmt =
> parse.parseTimestamp("%Y-%m-%d %H:%M:%S", "2024-01-01 00:00:00", "GMT");
> final long typo =
> parse.parseTimestamp("%Y-%m-%d %H:%M:%S", "2024-01-01 00:00:00",
> "Asia/Sanghai");
> assertThat(typo, is(gmt));
> }
> @Test void testParseTimestampRejectsUnknownTimeZone() {
> final SqlFunctions.DateParseFunction parse = new
> SqlFunctions.DateParseFunction();
> assertThrows(RuntimeException.class,
> () -> parse.parseTimestamp("%Y-%m-%d %H:%M:%S",
> "2024-01-01 00:00:00", "Asia/Sanghai"));
> } {code}
> Run:
> {code:java}
> ./gradlew :core:test \
> --tests
> org.apache.calcite.test.SqlFunctionsTest.testParseTimestampUnknownTimeZoneMatchesGmtFallback
> \
> --tests
> org.apache.calcite.test.SqlFunctionsTest.testParseTimestampRejectsUnknownTimeZone
> {code}
> Observed behavior:
> The fallback behavior is visible directly
> * parseTimestamp(..., "Asia/Sanghai") returns the same result as
> parseTimestamp(..., "GMT")
> * no exception is thrown for the unknown time-zone identifier.
> The rejecting test fails with
> {code:java}
> Expected java.lang.RuntimeException to be thrown, but nothing was thrown.
> {code}
> Expected behavior:
> An invalid time-zone identifier should be rejected, rather than being
> silently reinterpreted as GMT.
>
> This is a runtime input-validation bug. A misspelled time-zone argument
> changes the meaning of a query result while looking superficially valid,
> which is worse than a hard failure.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
