[jira] [Commented] (CALCITE-2082) Do not store types or type factories inside operators

["Ruben Q L (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-2082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17194318#comment-17194318 ] 

Ruben Q L commented on CALCITE-2082:
------------------------------------

Sorry to disturb you [~julianhyde], was there a PR for this patch?

I understand the motivation behind this modification, but could you please clarify the change on {{SqlUserDefinedFunction}}? Why we must now have a {{SqlOperandMetadata}} and not just any type {{SqlOperandTypeChecker}}? My application had several UDFs with {{SqlOperandTypeChecker}}s, and they already complied with "Do not store types (RelDataType) or type factories (RelDataTypeFactory) in SqlOperator instances", so I cannot understand why they should be impacted by this change and start using {{SqlOperandMetadata}}.

Also, please notice that this is a breaking change, all the (now) deprecated constructors of {{SqlUserDefinedFunction}}, {{SqlUserDefinedTableFunction}}, {{SqlUserDefinedAggFunction}} & {{SqlUserDefinedTableMacro}} will silently pass a {{null}} to {{super}}, instead of their (non {{SqlOperandMetadata}}) {{SqlOperandTypeChecker}}; which, if I am not mistaken, may have unpredictable consequences.

> Do not store types or type factories inside operators
> -----------------------------------------------------
>
>                 Key: CALCITE-2082
>                 URL: https://issues.apache.org/jira/browse/CALCITE-2082
>             Project: Calcite
>          Issue Type: Bug
>            Reporter: Julian Hyde
>            Assignee: Julian Hyde
>            Priority: Major
>             Fix For: 1.26.0
>
>
> Do not store types (RelDataType) or type factories (RelDataTypeFactory) in SqlOperator instances.
> *Rationale*: a {{SqlOperator}} has a lifetime that spans many statements; but a type factory is only for one statement, and each type belongs to that factory. We want to share {{SqlOperator}} instances across connections, therefore we need to create them before there is a type factory.
> Typically, a method that returns a type should have a type factory argument with which to create it.
> The current situation is technical debt. There are a couple of pieces of code tagged with this case number; see the fix to CALCITE-2072.
> In particular:
> * Remove method {{List<RelDataType> SqlOperator.getParamTypes()}};
> * Remove {{RelDataTypeFactory}} argument from {{SqlUserDefinedAggFunction}} constructor, and remove its {{typeFactory}} field.
> We will add {{interface SqlOperandMetadata extends SqlOperatorTypeChecker}}, which has new methods {{List<RelDataType>> paramTypes(RelDataTypeFactory)}} and {{List<String> paramNames()}}.
> This interface will typically be implemented only for user-defined functions. Unlike SQL built-in functions, UDFs have a fixed set of parameters (although some of them may be optional), and the parameters have names.
> In {{interface SqlOperandTypeChecker}}, add method {{boolean isFixedParameters()}}. Will typically return true for UDFs, false for built-in functions. Returns false for table window functions (e.g. {{HOP}}), even though these have named parameters (which tends to make them look a bit like UDFs).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)