[CALCITE-1882] Can't obtain the user defined aggregate function such …#502
[CALCITE-1882] Can't obtain the user defined aggregate function such …#502yuqi1129 wants to merge 1 commit intoapache:masterfrom
Conversation
| boolean requiresOrder, boolean requiresOver, RelDataTypeFactory typeFactory) { | ||
| super(Util.last(opName.names), opName, SqlKind.OTHER_FUNCTION, | ||
| super(Util.last(opName.names), opName, SqlKind.getSqlKindByName(opName.names.get(0)) != null | ||
| ? SqlKind.valueOf(opName.names.get(0)) : SqlKind.OTHER_FUNCTION, |
There was a problem hiding this comment.
I'm not sure this is the right approach. What you do is you make "user-defined SUM function" to be treated as standard SUM function without giving a single check on the actual implementation.
That might trigger some optimization rules that alter results. For instance, Calcite could replace user-defined AVG(...) with standard SUM(...)/COUNT(...).
There was a problem hiding this comment.
I just want to defined a user-defind agg function with the name SUM, for example, add a sum function of String type, however, the sql kind of this function is other function, which will be filtered when find the name of SUM, i can't find a better solution than this, if you have any idea about this, do not hesitate to help.
By the way, I am not very clear about your answer " without giving a single check on the actual implementation", could you explain it in detail ?
There was a problem hiding this comment.
I agree with @vlsi. It's OK to have a UDAF with the same name as a built-in, but it's not OK for it to impersonate it by taking on its SqlKind value.
What happens if you leave its SqlKind as OTHER_FUNCTION?
There was a problem hiding this comment.
@julianhyde @vlsi .I see, if this is not OK, i can change another method. The reason that causes this problem is that in parser phase, the parser directly convert 'SUM' to SqlSumAggFunction not SqlUnresolvedFuntion by name and at this time user-defined aggregate function has not been registered. this should be done in the validate phase as at this time converting 'Sum' to SqlSumAggFunction will cause calcite filters all function that is not SqlKind.SUM like user-defined aggregate funcion. Converting in the early phase may accelerate the validate phase later, but it will omit the user-defined aggregate function, which will lead validator can't find function.
My resolution:
Method of createCall in SqlAbstractParserImpl.java directly returns SqlUnresolvedFunction not a clear specific function like SqlSumAggFunction. But if do like this, hundreds of test case fail as createCall has changed and sql changed greatly after parser. this is quite troublesome.
Is that OK? advice is appreciate, thanks
There was a problem hiding this comment.
Let's discuss at https://issues.apache.org/jira/browse/CALCITE-1882.
…as sum,avg by calcite Closes apache#649
vlsi
added
help wanted
returned-with-feedback
|
I have the impression that after merging #1205 this PR is obsolete. If nobody objects I will close this PR and proceed on doing the same to the JIRA case. |
4 participants
…as sum,avg by calcite