[
https://issues.apache.org/jira/browse/CALCITE-2082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17194318#comment-17194318
]
Ruben Q L edited comment on CALCITE-2082 at 9/11/20, 3:15 PM:
--------------------------------------------------------------
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}}, 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.
was (Author: rubenql):
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)
