[
https://issues.apache.org/jira/browse/CALCITE-7559?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18084026#comment-18084026
]
Ruiqi Dong commented on CALCITE-7559:
-------------------------------------
Hi [~mbudiu]. Thanks for the pointer to CALCITE-7527! I'll follow the same
pattern and submit a PR.
> SqlParserUtil.parseTimeTzLiteral(...) accepts unknown time zones while
> sibling parseTimestampTzLiteral(...) rejects them
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-7559
> URL: https://issues.apache.org/jira/browse/CALCITE-7559
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.41.0
> Reporter: Ruiqi Dong
> Priority: Minor
>
> *Summary*
> parseTimestampTzLiteral(...) already validates zone identifiers via
> ZoneId.of(...), but parseTimeTzLiteral(...) still uses
> TimeZone.getTimeZone(...) without validating the input.
> Because TimeZone.getTimeZone(...) silently falls back to GMT for unknown IDs,
> invalid "TIME WITH TIME ZONE" literals are accepted instead of rejected. The
> sibling "TIMESTAMP WITH TIME ZONE" parser rejects the same bad zone string,
> so the inconsistency is inside SqlParserUtil itself rather than a broader
> parser policy.
>
> *Affected code*
> File: core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java
> {code:java}
> public static SqlTimeTzLiteral parseTimeTzLiteral(
> String s, SqlParserPos pos) {
> final int lastSpace = s.lastIndexOf(" ");
> DateTimeUtils.PrecisionTime pt = null;
> if (lastSpace >= 0) {
> final String timeZone = s.substring(lastSpace + 1);
> final String time = s.substring(0, lastSpace);
> final TimeZone tz = TimeZone.getTimeZone(timeZone);
> if (tz != null) {
> pt =
> DateTimeUtils.parsePrecisionDateTimeLiteral(time,
> Format.get().time, tz, -1);
> }
> }
> if (pt == null) {
> throw SqlUtil.newContextException(pos,
> RESOURCE.illegalLiteral("TIME WITH TIME ZONE", s,
> RESOURCE.badFormat(DateTimeUtils.TIME_FORMAT_STRING).str()));
> }
> ...
> } {code}
> {code:java}
> public static SqlTimestampTzLiteral parseTimestampTzLiteral(
> String s, SqlParserPos pos) {
> ...
> try {
> ZoneId zoneId = ZoneId.of(timeZone);
> TimeZone tz = TimeZone.getTimeZone(zoneId);
> ...
> } catch (DateTimeException e) {
> throw SqlUtil.newContextException(pos,
> RESOURCE.illegalLiteral("TIMESTAMP WITH TIME ZONE", s, message));
> }
> } {code}
> Unlike parseTimestampTzLiteral(...), the "TIME WITH TIME ZONE" path never
> checks whether the zone identifier is actually valid. The if (tz != null)
> guard does not help here, because an unknown zone is converted into GMT
> instead of yielding null.
>
> *Reproducer*
> Add the following test to
> core/src/test/java/org/apache/calcite/sql/parser/SqlParserUtilTest.java
> {code:java}
> @Test void testTimeWithTimeZoneRejectsUnknownTimeZone() {
> SqlParserPos pos = new SqlParserPos(2, 3);
> assertThrows(CalciteContextException.class,
> () -> SqlParserUtil.parseTimeTzLiteral("10:10:10 incorrect_zone", pos));
> } {code}
> Run:
> {code:java}
> ./gradlew -q :core:test --tests
> org.apache.calcite.sql.parser.SqlParserUtilTest.testTimeWithTimeZoneRejectsUnknownTimeZone
> {code}
> Observed behavior:
> The test fails because no exception is thrown
> {code:java}
> Expected org.apache.calcite.runtime.CalciteContextException to be thrown, but
> nothing was thrown. {code}
> Expected behavior:
> An invalid time-zone identifier should be rejected, just as invalid zones are
> already rejected for "TIMESTAMP WITH TIME ZONE".
>
> The same malformed zone identifier is rejected by the sibling "TIMESTAMP WITH
> TIME ZONE" path but accepted here and reinterpreted under GMT. That silently
> changes the meaning of the literal instead of reporting bad input.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
