[
https://issues.apache.org/jira/browse/CALCITE-2670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16688817#comment-16688817
]
Hongze Zhang edited comment on CALCITE-2670 at 11/16/18 12:06 AM:
------------------------------------------------------------------
bq. Why did you convert some parameters from particular types (e.g.
SqlJsonQueryEmptyOrErrorBehavior) to untyped Enum?
Thanks [~julianhyde] and I missed explaining this point.
In the patch, I have added SYMBOL-to-Enum type transform to
JavaTypeFactoryImpl#getJavaClass. That is, the built-in rel implementer does
not know the original type of a SYMBOL.
And rel implementer applies explicitly type casting when translating SYMBOL
parameter that is from an aggregate call, so if we transform SYMBOL to Enum in
JavaTypeFactoryImpl#getJavaClass, we can only use Enum for the related methods
in SqlFunctions.java.
It works before because there were methods like
SqlFunctions#jsonObjectAggAddNullOnNull that is for the specific operator like
JSON_OBJECTAGG_NULL_ON_NULL, I have deleted these methods in the patch.
And SYMBOL-to-Enum type casting is a trade-off - If we use a JavaRecordType to
carry a particular enum type instead of using BasicSqlType, we could use
particular types in SqlFunctions.java. But this way needs more code changes,
and JavaRecordType is still experimental and not recommended for use.
was (Author: zhztheplayer):
bq. Why did you convert some parameters from particular types (e.g.
SqlJsonQueryEmptyOrErrorBehavior) to untyped Enum?
Thanks [~julianhyde] and I missed explaining this point.
In the patch, I have added SYMBOL-to-Enum type transform to
JavaTypeFactoryImpl#getJavaClass. That is, the built-in rel implementer does
not know the original type of a SYMBOL.
And rel implementer applies explicitly type casting when translating SYMBOL
parameter that is from an aggregate call, so if we transform SYMBOL to Enum in
JavaTypeFactoryImpl#getJavaClass, we can only use Enum for the related methods
in SqlFunctions.java.
It works before because there were methods like
SqlFunctions#jsonObjectAggAddNullOnNull that is for the specific operator like
JSON_OBJECTAGG_NULL_ON_NULL, I have deleted these methods in the patch.
And SYMBOL-to-Enum type casting is a trade-off - If we use a JavaRecordType to
carry a particular enum type instead of using RelDataType, we could use
particular types in SqlFunctions.java. But this way needs more code changes,
and JavaRecordType is still experimental and not recommended for use.
> 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)
