dssysolyatin commented on PR #4696: URL: https://github.com/apache/calcite/pull/4696#issuecomment-3675642290
@mihaibudiu You decided not to wait for the PR and did the work yourself: https://github.com/apache/calcite/pull/4644. That’s totally understandable. I’m sorry I couldn’t finish my PR in time, so I understand why you moved forward. If possible, please use SqlBasicOperator. See the discussion here: https://issues.apache.org/jira/browse/CALCITE-5403 (especially this [comment](https://issues.apache.org/jira/browse/CALCITE-5403?focusedCommentId=17710252&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17710252 )) > Having a subclass of SqlSpecialOperator for every kind of Ast node is a pattern that we need to stop doing. Can you find a way to do this using SqlBasicOperator or similar? If method is private: ``` private static final SqlSpecialOperator OPERATOR ``` we can change `SqlSpecialOperator` to `SqlOperator` ``` private static final SqlOperator OPERATOR ``` and use SqlBasicOperator instead. See example inside https://github.com/apache/calcite/pull/4644 Regarding SqlAttributeDefinition: 1. I don’t see where `SqlCollation` is used for `SqlAttributeDefinition`. In the parser, [the only place](https://github.com/apache/calcite/blob/23b7931c3e516bdb6cfedda956213f7fe06c6b24/server/src/main/codegen/includes/parserImpls.ftl#L219) that references it passes null for `SqlAttributeDefinition`. So, I assume nobody uses it. 2. If we decide to support it later, I think SqlCollation should extend SqlNode directly. It already has an unparse method. For Coercibility, you can use SqlLiteral.createSymbol(). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
