You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Muhammad Gelbana <m....@gmail.com> on 2019/06/08 21:10:45 UTC
Is it essential to unparse to the same original syntax ?
I created a PR [1] to support the PostgreSQL :: casting operator. The way I
did this is by creating a new 'SqlBinaryOperator' child. This new child
wraps an instance of the 'SqlCastFunction' to reuse it's
'getOperandCountRange',
'inferReturnType', 'checkOperandTypes' and 'getMonotonicity' logic, and of
course unparses to the original input (i.e. op1 :: type).
But then the PR was commented to reuse the 'SqlCastFunction' type instead
of having a totally new 'SqlBinaryOperator', wich won't unparse properly
because 'op1 :: type' will be unparsed as 'CAST(op1 AS type)'.
Is this a big deal ? I prefer to preserve the orignal format for the parsed
string but to do that I'll have to extend 'SqlCastFunction' to override
it's 'unparse' implementation (I don't remember why I didn't do that, the
PR is like 3 months old)
So is preserving the original structure necessary, recommended or a must
while unparsing ?
If there are any related restriction I need to follow while working on
this, please let me know.
[1] https://github.com/apache/calcite/pull/1066
Thanks,
Gelbana
Re: Is it essential to unparse to the same original syntax ?
Posted by Julian Hyde <jh...@apache.org>.
Since the "::" operator is of a different syntactic type to the CAST
operator it probably makes sense to have two operators.
I often say that it is wrong to make the parser do work other than
just parsing. In the case of :: and CAST, I suspect that there are
differences in semantics. If we make the parser represent :: as CAST,
we might end up asking the parser to make decisions that it isn't
equipped to make.
On Mon, Jun 10, 2019 at 4:10 AM Lai Zhou <hh...@gmail.com> wrote:
>
> @Muhammad Gelban , since the SqlStdOperatorTable.CAST is hard coded , I
> think it's difficult to replace it absolutely with a new cast function now.
>
> Lai Zhou <hh...@gmail.com> 于2019年6月10日周一 下午6:54写道：
>
> > @Muhammad Gelban, I suggest you to find the the usages of an Operator
> > first , if you want to define a new one to replace it .
> > In this case, you will find that the SqlStdOperatorTable.CAST is used by
> > the Sql Converter, there're a few more places need attention.
> > You can refer to the solution of my custom sql engine , that shows how to
> > make a HiveSqlCastFunction to replace the default one.
> >
> > https://github.com/51nb/marble/blob/master/marble-table-hive/src/main/java/org/apache/calcite/adapter/hive/HiveSqlCastFunction.java
> >
> > https://github.com/51nb/marble/blob/83f0ea2941affbaf792a6290963ffc0b5277512f/marble-table-hive/src/main/java/org/apache/calcite/adapter/hive/HiveConvertletTable.java#L44
> >
> >
> >
> >
> >
> > Muhammad Gelbana <m....@gmail.com> 于2019年6月9日周日 上午5:11写道：
> >
> >> I created a PR [1] to support the PostgreSQL :: casting operator. The way
> >> I
> >> did this is by creating a new 'SqlBinaryOperator' child. This new child
> >> wraps an instance of the 'SqlCastFunction' to reuse it's
> >> 'getOperandCountRange',
> >> 'inferReturnType', 'checkOperandTypes' and 'getMonotonicity' logic, and of
> >> course unparses to the original input (i.e. op1 :: type).
> >>
> >> But then the PR was commented to reuse the 'SqlCastFunction' type instead
> >> of having a totally new 'SqlBinaryOperator', wich won't unparse properly
> >> because 'op1 :: type' will be unparsed as 'CAST(op1 AS type)'.
> >>
> >> Is this a big deal ? I prefer to preserve the orignal format for the
> >> parsed
> >> string but to do that I'll have to extend 'SqlCastFunction' to override
> >> it's 'unparse' implementation (I don't remember why I didn't do that, the
> >> PR is like 3 months old)
> >>
> >> So is preserving the original structure necessary, recommended or a must
> >> while unparsing ?
> >> If there are any related restriction I need to follow while working on
> >> this, please let me know.
> >>
> >> [1] https://github.com/apache/calcite/pull/1066
> >>
> >> Thanks,
> >> Gelbana
> >>
> >
Re: Is it essential to unparse to the same original syntax ?
Posted by Lai Zhou <hh...@gmail.com>.
@Muhammad Gelban , since the SqlStdOperatorTable.CAST is hard coded , I
think it's difficult to replace it absolutely with a new cast function now.
Lai Zhou <hh...@gmail.com> 于2019年6月10日周一 下午6:54写道：
> @Muhammad Gelban, I suggest you to find the the usages of an Operator
> first , if you want to define a new one to replace it .
> In this case, you will find that the SqlStdOperatorTable.CAST is used by
> the Sql Converter, there're a few more places need attention.
> You can refer to the solution of my custom sql engine , that shows how to
> make a HiveSqlCastFunction to replace the default one.
>
> https://github.com/51nb/marble/blob/master/marble-table-hive/src/main/java/org/apache/calcite/adapter/hive/HiveSqlCastFunction.java
>
> https://github.com/51nb/marble/blob/83f0ea2941affbaf792a6290963ffc0b5277512f/marble-table-hive/src/main/java/org/apache/calcite/adapter/hive/HiveConvertletTable.java#L44
>
>
>
>
>
> Muhammad Gelbana <m....@gmail.com> 于2019年6月9日周日 上午5:11写道：
>
>> I created a PR [1] to support the PostgreSQL :: casting operator. The way
>> I
>> did this is by creating a new 'SqlBinaryOperator' child. This new child
>> wraps an instance of the 'SqlCastFunction' to reuse it's
>> 'getOperandCountRange',
>> 'inferReturnType', 'checkOperandTypes' and 'getMonotonicity' logic, and of
>> course unparses to the original input (i.e. op1 :: type).
>>
>> But then the PR was commented to reuse the 'SqlCastFunction' type instead
>> of having a totally new 'SqlBinaryOperator', wich won't unparse properly
>> because 'op1 :: type' will be unparsed as 'CAST(op1 AS type)'.
>>
>> Is this a big deal ? I prefer to preserve the orignal format for the
>> parsed
>> string but to do that I'll have to extend 'SqlCastFunction' to override
>> it's 'unparse' implementation (I don't remember why I didn't do that, the
>> PR is like 3 months old)
>>
>> So is preserving the original structure necessary, recommended or a must
>> while unparsing ?
>> If there are any related restriction I need to follow while working on
>> this, please let me know.
>>
>> [1] https://github.com/apache/calcite/pull/1066
>>
>> Thanks,
>> Gelbana
>>
>
Re: Is it essential to unparse to the same original syntax ?
Posted by Lai Zhou <hh...@gmail.com>.
@Muhammad Gelban, I suggest you to find the the usages of an Operator
first , if you want to define a new one to replace it .
In this case, you will find that the SqlStdOperatorTable.CAST is used by
the Sql Converter, there're a few more places need attention.
You can refer to the solution of my custom sql engine , that shows how to
make a HiveSqlCastFunction to replace the default one.
https://github.com/51nb/marble/blob/master/marble-table-hive/src/main/java/org/apache/calcite/adapter/hive/HiveSqlCastFunction.java
https://github.com/51nb/marble/blob/83f0ea2941affbaf792a6290963ffc0b5277512f/marble-table-hive/src/main/java/org/apache/calcite/adapter/hive/HiveConvertletTable.java#L44
Muhammad Gelbana <m....@gmail.com> 于2019年6月9日周日 上午5:11写道：
> I created a PR [1] to support the PostgreSQL :: casting operator. The way I
> did this is by creating a new 'SqlBinaryOperator' child. This new child
> wraps an instance of the 'SqlCastFunction' to reuse it's
> 'getOperandCountRange',
> 'inferReturnType', 'checkOperandTypes' and 'getMonotonicity' logic, and of
> course unparses to the original input (i.e. op1 :: type).
>
> But then the PR was commented to reuse the 'SqlCastFunction' type instead
> of having a totally new 'SqlBinaryOperator', wich won't unparse properly
> because 'op1 :: type' will be unparsed as 'CAST(op1 AS type)'.
>
> Is this a big deal ? I prefer to preserve the orignal format for the parsed
> string but to do that I'll have to extend 'SqlCastFunction' to override
> it's 'unparse' implementation (I don't remember why I didn't do that, the
> PR is like 3 months old)
>
> So is preserving the original structure necessary, recommended or a must
> while unparsing ?
> If there are any related restriction I need to follow while working on
> this, please let me know.
>
> [1] https://github.com/apache/calcite/pull/1066
>
> Thanks,
> Gelbana
>