[
https://issues.apache.org/jira/browse/CALCITE-3010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16822865#comment-16822865
]
Hongze Zhang edited comment on CALCITE-3010 at 4/22/19 4:49 AM:
----------------------------------------------------------------
Thank you very much for the review, Julian!
Address your comment:
{quote}Do you agree that it is an improvement?
{quote}
Yes, I agreed. At the very first I though one thing that may block this fix is
about JSON function's operand validation. Currently quiet a few of the
functions require a {{JsonValueExpression}} as the first argument. And per
definition in code, return value of the expression is of {{SqlTypeName.ANY}}
type. Say if we have something like {{SqlTypeName.JSON}} type already defined,
it should be more straightforward to validate the functions' operands within
the new added type. Otherwise the validator won't easily distinguish whether
the argument is a {{JsonValueExpression}}, or some other expressions of the
type {{SqlTypeName.ANY}}.
Fortunately after giving it a try (the PR), it looks to be very reasonable to
me to fix this issue before actually adding the new type. The two problems can
be separated if we deal with them carefully. And of course I'll make another PR
fixing CALCITE-2869 if no one else is currently working on that.
was (Author: zhztheplayer):
Thank you very much for the review, Julian!
Address your comment:
{quote}Do you agree that it is an improvement?
{quote}
Yes, I agreed. At the very first I though one thing that may block this fix is
about JSON function's operand validation. Currently quiet a few of the
functions require a {{JsonValueExpression}} as the first argument. And per
definition in code, return value of the expression is of {{SqlTypeName.ANY}}
type. Say if we have something like {{SqlTypeName.JSON}} type already defined,
it should be more straightforward to validate the functions' operands within
the new added type. Otherwise the validator won't easily distinguish whether
the argument is a {{JsonValueExpression}}, or some other expressions of the
type {{SqlTypeName.ANY}}.
Fortunately after giving it a try (the PR), it looks to be very reasonable to
me to fix this issue before actually adding the new type. The two problems can
be separated if we deal with them carefully. And of curse I'll make another PR
fixing CALCITE-2869 if no one else is currently working on that.
> In SQL parser, move JsonValueExpression into Expression
> -------------------------------------------------------
>
> Key: CALCITE-3010
> URL: https://issues.apache.org/jira/browse/CALCITE-3010
> Project: Calcite
> Issue Type: Sub-task
> Reporter: Hongze Zhang
> Assignee: Hongze Zhang
> Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Below is the syntax definition of {{JsonValueExpression}} from ISO/SEC
> 19075-6:
> {code:java}
> <JSON value expression> ::=
> <value expression> [ <JSON input clause> ]
> <JSON input clause> ::=
> FORMAT <JSON representation>
> <JSON representation> ::=
> JSON [ ENCODING { UTF8 | UTF16 | UTF32 } ]
> | <implementation-defined JSON representation option>
> {code}
> Currently it's an individual syntax standing out of {{Expression}} for easily
> implementing the behavior of "implicit JSON format". As the amount of JSON
> functions is getting larger, the design could definitely be improved. For
> example, it can be categorized as a PostfixOperator[1].
> [1]
> [https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/sql/SqlPostfixOperator.java]
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
