[
https://issues.apache.org/jira/browse/CALCITE-2670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16691560#comment-16691560
]
Hongze Zhang edited comment on CALCITE-2670 at 11/19/18 11:08 AM:
------------------------------------------------------------------
{quote}If it's data, it has to follow the SQL type system, and of course you
can't figure out a more specific type than Enum. But should it be data? I think
the argument should be hard-coded into the call.
{quote}
Thanks [~julianhyde] and I agree that hard-coded argument is better. But I am
not sure if there is any proper way to pass a non-data argument to an aggregate
call without modifing massive code when implementing the plan. Now
AggregateCall supports integer input ref list as the argument list, rather than
a rex node list. Which means, if we make each flag an argument, the argment
will be convert to data by the implementor. If there is any good way to avoid
this please let me know.
Now Calcite's JSON_OBJECTAGG has only one flag - the error behavior, but the
function has 2 flags in SQL standard. If using different operators to represent
the same function with different flags, when flag count is up to 2, 3 ,4 or
more, registered operator count will be a lot. (e.g.
JSON_OBJECTAGG_NULL_ON_NULL_WITH_UNIQUE_KEY,
JSON_OBJECTAGG_NULL_ON_NULL_WITHOUT_UNIQUE_KEY,
JSON_OBJECTAGG_ERROR_ON_NULL_WITH_UNIQUE_KEY... ), so I believe it is neccesary
to combine such operators. If leaving them separated is OK, please also let me
know.
was (Author: zhztheplayer):
bq. If it's data, it has to follow the SQL type system, and of course you can't
figure out a more specific type than Enum. But should it be data? I think the
argument should be hard-coded into the call.
Thanks [~julianhyde] and I agree that hard-coded argument is better. But I am
not sure if there is any proper way to pass a non-data argument to an aggregate
call without modifing massive code when implementing the plan. Now
AggregateCall supports integer input ref list as the argument list, rather than
a rex node list. Which means, if we make each flag an argument, the argment
will be convert to data by the implementor. If there is any good way to avoid
this please let me know.
Now Calcite's JSON_OBJECTAGG has only one flag - the error behavior, but the
function has 2 flags in SQL standard. If using different operators to represent
the same function with different flags, when flag count is up to 2, 3 ,4 or
more, registered operator count will be a lot. (e.g.
JSON_OBJECTAGG_NULL_ON_NULL_WITH_UNIQUE_KEY,
JSON_OBJECTAGG_NULL_ON_NULL_WITHOUT_UNIQUE_KEY,
JSON_OBJECTAGG_ERROR_ON_NULL_WITH_UNIQUE_KEY... ), so I believe it is neccesary
to combine such operators.
> Combine similar JSON aggregate functions in operator table
> ----------------------------------------------------------
>
> Key: CALCITE-2670
> URL: https://issues.apache.org/jira/browse/CALCITE-2670
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Hongze Zhang
> Assignee: Julian Hyde
> Priority: Major
>
> In current code master of Calcite, operator *JSON_ARRAYAGG* and
> *JSON_OBJECTAGG* are separated to *JSON_ARRAYAGG_NULL_ON_NULL*,
> *JSON_ARRAYAGG_ABSENT_ON_NULL*, *JSON_OBJECTAGG_NULL_ON_NULL* and
> *JSON_OBJECTAGG_ABSENT_ON_NULL* in SqlStdOperatorTable.java. This is OK for
> now, but may deliver more difficulty on extending syntax of *JSON_ARRAYAGG*
> and *JSON_OBJECTAGG* and on implementing logical plan by adapters.
> This is basically to combine the 4 operators to 2, this is a improvement list:
> # Combine *JSON_ARRAYAGG_NULL_ON_NULL* and *JSON_ARRAYAGG_ABSENT_ON_NULL;*
> # Combine *JSON_OBJECTAGG_NULL_ON_NULL* and *JSON_OBJECTAGG_ABSENT_ON_NULL;*
> # *SYMBOL* type Support for *JavaTypeFactoryImpl#getJavaClass*
> This is to generate *Enum* java type for *SYMBOL*. Current version of
> Calcite generates *Object[]*, which delivers type casting error, or some
> method compatible problems {color:#333333}when Calcite tries to pass enum
> parameters to an aggregate function;{color}
> # Add SQL-to-Rel test cases for JSON functions.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
