dssysolyatin commented on PR #4696:
URL: https://github.com/apache/calcite/pull/4696#issuecomment-3677903533

   The main recommendation is to use `SqlBasicOperator` instead of making new 
subclasses of `SqlSpecialOperator`. Example 
https://github.com/apache/calcite/pull/4644/files#diff-0e0b4e3b10ec6fc8fcd1910b4d2b070e0c0b5a62d5bd75befbd46d3d7d33712c
   ```java
    private static final SqlOperator OPERATOR =
         SqlBasicOperator.create("ATTRIBUTE_DEF", SqlKind.ATTRIBUTE_DEF)
             .withCallFactory((operator, functionQualifier, pos, operands) ->
                 new SqlAttributeDefinition(
                     pos,
                     (SqlIdentifier) requireNonNull(operands[0]),
                     (SqlDataTypeSpec) requireNonNull(operands[1]),
                     operands[2], null));
   ```
   > I tried to use these classes and I couldn't.
   
   Do you mean you tried `SqlBasicOperator` and it didn’t work as expected?
   
   > to switch to use SqlOperator everywhere?
   
   In current cases, all `OPERATOR`s are private (For public properties, it 
would be better to use just `SqlOperator` to avoid possible compatibility 
issues in the future). Therefore, if SqlBasicOperator is not used, there is no 
real need to change this.
   
   > One can convert enums to SqlLiterals with string values. But how about the 
Locale in the Collation?
   
   Is a simple string insufficient to represent a Locale and Collation? I think 
it want one of the constructor does:
   ```java
   public SqlCollation(
         @JsonProperty("collationName") String collation,
         @JsonProperty("coercibility") Coercibility coercibility) {
   ```
   


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

Reply via email to