[ https://issues.apache.org/jira/browse/CALCITE-6092?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17783429#comment-17783429 ]
Jerin John edited comment on CALCITE-6092 at 11/7/23 12:54 AM: --------------------------------------------------------------- Makes sense [~julianhyde], seems like these test outputs were replaced with their existing returned values without checking for validity in some cases. This failure surfaced when we synced latest changes from Calcite into [Looker's fork|https://github.com/looker-open-source/calcite], I traced the history and found {{SqlOperatorTests}} started failing since these [commit|https://github.com/apache/calcite/commit/f996bc9993546019d0fd475b2daa99ac8b1b9259] changes. This same error was reported in CALCITE-5957 but I think only the [cast to TIMESTAMP|https://github.com/apache/calcite/commit/f996bc9993546019d0fd475b2daa99ac8b1b9259#diff-fdd0c725fc7c6fd56965f59b1d51e4c7a9b5b5be27da2e54b8b8273dc980cd64R1262-R1263] cases were handled as an exception, should we include these cast to TIME tests with same inputs under the same [BUG condition|https://github.com/apache/calcite/commit/f996bc9993546019d0fd475b2daa99ac8b1b9259#diff-fdd0c725fc7c6fd56965f59b1d51e4c7a9b5b5be27da2e54b8b8273dc980cd64R1257-R1258], so that based on Avatica version they get handled correctly? I'm guessing based on this [commit|https://github.com/apache/calcite-avatica/commit/05e07c151a47e5225b67084abb2e2b9fe161a30d] this time string should be invalidated correctly when avatica 1.24.0 comes out? was (Author: JIRAUSER301314): Makes sense [~julianhyde], seems like these test outputs were replaced with their existing returned values without checking for validity in some cases. This failure surfaced when we synced latest changes from Calcite into [Looker's fork|https://github.com/looker-open-source/calcite], I traced the history and found {{SqlOperatorTests}} started failing since these [commit|https://github.com/apache/calcite/commit/f996bc9993546019d0fd475b2daa99ac8b1b9259] changes. This same error was reported in CALCITE-5957 but I think only the [cast to TIMESTAMP|https://github.com/apache/calcite/commit/f996bc9993546019d0fd475b2daa99ac8b1b9259#diff-fdd0c725fc7c6fd56965f59b1d51e4c7a9b5b5be27da2e54b8b8273dc980cd64R1262-R1263] cases were handled as an exception, should include these cast to TIME tests with same inputs under the same [BUG condition|https://github.com/apache/calcite/commit/f996bc9993546019d0fd475b2daa99ac8b1b9259#diff-fdd0c725fc7c6fd56965f59b1d51e4c7a9b5b5be27da2e54b8b8273dc980cd64R1257-R1258] > Invalid test cases in CAST String to Time > ----------------------------------------- > > Key: CALCITE-6092 > URL: https://issues.apache.org/jira/browse/CALCITE-6092 > Project: Calcite > Issue Type: Bug > Reporter: Jerin John > Priority: Major > > Encountered some > [tests|https://github.com/apache/calcite/blob/590ec85f0fcff7173c288c350c3f60e640976a34/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java#L1237-L1238] > within SqlOperatorTest file for the CAST operator on String to Datetime > conversions, which are found to be invalid time strings on some tested > instances of DBs like BQ, MySql, Postgres. > It seems these tests were originally ignored as `BAD_DATETIME_MESSAGE` but > then updated to verify conversion performed by the CAST operator (refer to > JIRA ticket: CALCITE-5554 and > [commit|https://github.com/apache/calcite/commit/625a2e03c4c5583279350bf04e3db2a31b1ec411#diff-fdd0c725fc7c6fd56965f59b1d51e4c7a9b5b5be27da2e54b8b8273dc980cd64R1118-R1119]) > Example test case (L1223): > {{cast('1241241' as TIME)}} | expected: "72:40:12" | actual: "16:40:12" > The string '1241241' is being parsed as number of hours, and internal > conversion into milliseconds is done as {{{}1241241 * (int) MILLIS_PER_HOUR > }} which results in na INT overflow. > The resultant overflowed value is 1701612160 milliseconds = 472.670044444 > hours, and 472 % 24 is 16 (and .67 is roughly 2/3 so 40ish minutes seems > legit) > Considering that multiple dialects catch these cases as invalid time strings > for conversion, code should be updated to handle them as exceptions and the > tests to be corrected to reflect this behavior. -- This message was sent by Atlassian Jira (v8.20.10#820010)