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/10/18 20:44:27 UTC

[PR] [CALCITE-6055] Customize handling of type name based on type system [calcite]

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

   Outline of the approach:
   - Treat `TIMESTAMP`, `TIMESTAMP WITH LOCAL TIME ZONE`, and `DATETIME` type names as user-defined types during parsing.
   - When deriving the types of these nodes, look for mappings in the schema model as before, but allow user-defined types to fall back on canonical types if no mapping is found, much like how `SqlUnknownLiteral` works. This is important so the `TIMESTAMP` and `TIMESTAMP WITH LOCAL TIME ZONE` types can still function when there is no UDT mapping for them.
   
   There's still a lingering problem in how to unparse the `SqlUserDefinedTypeNameSpec` nodes. This is only relevant when a `SqlNode` AST is parsed and then unparsed without converting to `RelNode` form in between. I describe why I don't think this is a particularly important use-case in [this Jira comment](https://issues.apache.org/jira/browse/CALCITE-5526?focusedCommentId=17776449&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17776449), but it's how the `SqlParserTest` and it's various subclasses operate. Looking for feedback on how to better address this:
   
   - The most obvious solution seems like implementing `unparse()` in a way similar to `deriveType()`: look up the UDT mapping and fall back on the standard types, thus preserving the existing default unparsing behavior for `TIMESTAMP`. However, this would require passing a `SqlValidator` to `unparse()`, which doesn't make a ton a sense.
   - We could instead ignore the UDT map during unparsing, but still try to match the type name to the standard `SqlTypeName`, so `TIMESTAMP` unparsing is unaffected by default. However, this would mess up unparsing a dialect like BigQuery, since BigQuery timestamps would unparse as `TIMESTAMP WITH LOCAL TIME ZONE`, and datetimes would unparse as `` `DATETIME` `` (with backticks).
   
   There are myriad details that could be tweaked in either case, and possibly other approached I'm not thinking of yet.


-- 
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-6055] Customize handling of type name based on type system [calcite]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3476:
URL: https://github.com/apache/calcite/pull/3476#issuecomment-2533560480

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.


-- 
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