You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "wnob (via GitHub)" <gi...@apache.org> on 2023/04/11 23:09:20 UTC

[GitHub] [calcite] wnob opened a new pull request, #3144: [CALCITE-5636] Consult root schema type map for coercion during validation

wnob opened a new pull request, #3144:
URL: https://github.com/apache/calcite/pull/3144

   Always look in the root schema's type map when inserting a `CAST` for automatic type coercion.
   
   Other things to note:
   - I modified `SqlFunctions.toTimestampWithLocalTimeZone(String)`, which is only invoked by the rex-to-lix translator to convert strings to `TIMESTAMP WITH LOCAL TIME ZONE` values, such as during coercion or casting in general. The method was implemented to address [CALCITE-1947](https://issues.apache.org/jira/browse/CALCITE-1947) but requires an explicit time zone component in the string literal to avoid a string-index-out-of-bounds exception in the constructor for `TimestampWithTimeZoneString`, which was originally named `TimestampWithLocalTimeZoneString` but renamed after the discussion in the Jira ticket to avoid confusion. It appears the problem with the name was address but the problem of semantics remains; one cannot cast a string without an explicit time zone to a timestamp w/ LTZ. I added a fallback that invokes `DateTimeUtils.timestampStringToUnixDate()`, which is the same function invoked by the rex-to-lix translator to convert strings to regular `TIMESTAMP` values. This
  effectively assumes UTC for all timestamp w/ LTZ conversions lacking an explicit time zone. Currently looking into getting the proper zone out of the `DataContext` but having trouble figuring out where it gets passed in.
   - I set the default time zone for the BQ Quidem test to UTC, which causes `TIMESTAMP WITH LOCAL TIME ZONE` to assume UTC for the coercion of strings without an explicit time zone component (this is consistent with how BigQuery actually behaves).


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CALCITE-5636] Consult root schema type map for coercion during validation [calcite]

Posted by "wnob (via GitHub)" <gi...@apache.org>.
wnob commented on PR #3144:
URL: https://github.com/apache/calcite/pull/3144#issuecomment-1787970534

   This was barking up the wrong tree. A better solution to this problem (currently a draft) is here: https://github.com/apache/calcite/pull/3476


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CALCITE-5636] Consult root schema type map for coercion during validation [calcite]

Posted by "wnob (via GitHub)" <gi...@apache.org>.
wnob closed pull request #3144: [CALCITE-5636] Consult root schema type map for coercion during validation
URL: https://github.com/apache/calcite/pull/3144


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] julianhyde commented on pull request #3144: [CALCITE-5636] Consult root schema type map for coercion during validation

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on PR #3144:
URL: https://github.com/apache/calcite/pull/3144#issuecomment-1505731593

   Let's discuss in the Jira case. I am not convinced that this is a bug. I see a bunch of code changes you want to make, but I don't see a high-level rationale.


-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [calcite] julianhyde commented on a diff in pull request #3144: [CALCITE-5636] Consult root schema type map for coercion during validation

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3144:
URL: https://github.com/apache/calcite/pull/3144#discussion_r1164491802


##########
babel/src/test/resources/sql/big-query.iq:
##########
@@ -2152,24 +2152,79 @@ SELECT
 
 !ok
 
-# The following example shows the result of DATETIME_DIFF for two days
-# in succession. The first date falls on a Monday and the second date
-# falls on a Sunday. DATETIME_DIFF with the date part WEEK returns 0
-# because this time part uses weeks that begin on
-# Sunday. DATETIME_DIFF with the date part WEEK(MONDAY) returns
-# 1. DATETIME_DIFF with the date part ISOWEEK also returns 1 because
-# ISO weeks begin on Monday.
-
-SELECT
-  DATETIME_DIFF('2017-12-18', '2017-12-17', WEEK) AS week_diff,
-  DATETIME_DIFF('2017-12-18', '2017-12-17', WEEK(MONDAY)) AS week_weekday_diff,
-  DATETIME_DIFF('2017-12-18', '2017-12-17', ISOWEEK) AS isoweek_diff;
+# The following example shows the result of DATETIME_DIFF for many nearby dates.
+# 2017-12-18 is a Monday and the other date is between 2017-12-25 (a Friday)
+# and 2017-12-09 (a Saturday). The date part WEEK treats weeks as starting on
+# Sunday, whereas WEEK(MONDAY) and ISOWEEK both treat weeks as starting on
+# Monday.
+
+WITH Diffs AS (

Review Comment:
   Let's stop writing this style of test. UNION ALL doesn't guarantee ordering.
   
   Use a CTE with the input values - DATE '2017-12-18', DATE '2017-12-25' etc. - and change the query to print the input value of each row.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org