[ 
https://issues.apache.org/jira/browse/CALCITE-7559?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Zhen Chen reassigned CALCITE-7559:
----------------------------------

    Assignee: Ruiqi Dong

> 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
>            Assignee: Ruiqi Dong
>            Priority: Minor
>              Labels: pull-request-available
>             Fix For: 1.43.0
>
>
> *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)

Reply via email to