[
https://issues.apache.org/jira/browse/CALCITE-6702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17901134#comment-17901134
]
Dhia Eddine Nini edited comment on CALCITE-6702 at 11/26/24 9:15 AM:
---------------------------------------------------------------------
{color:#7a869a}Now I am worrying that there are many more functions that have
the same problem - ATAN2, for example - and how we could detect and fix these
functions efficiently.{color}
* [~julianhyde] I agree with you that there might be many more functions that
have the same problem, +i.e.+ having wrong {{Strong }}policy assignment, though
their null policy seems to be correct in {{{}RexImpTable{}}}.
The problem is that, like [~mbudiu] mentioned, we are not using the values in
{{RexImpTable}} in order to simplify {{{}RexNodes{}}}, which leads to operators
like {{*POWER(base, exp)*}} slipping through those simplifications. My question
in this case would be: What are the differences between {{Strong.Policy}} and
{{{}NullPolicy{}}}in Apache Calcite? Is it that the{{ Strong.Policy}} is used
strictly for {*}predicates{*}, while {{NullPolicy }}is used for
{*}SqlOperators{*}? Can't we unify their respective definitions somehow since
they overlap in functionality, no?
{quote}My question to you is: What evidence did you see that made you realize
that POWER should be ANY, not AS_IS? What is the test case that passes only
after this bug is fixed? I want to apply that test case automatically to all
functions.
{quote} * The test case that only passes after this bug is fixed is when I
convert the following SQL query to a RelNode and then assert on the output
RelNode. This is what I get:
!image-2024-11-26-11-11-18-721.png!
* With regards to automating this test case to apply to all functions, maybe
we can iterate over all the preciates / operators that we deem to have a
{{Strong.Policy.ANY}} value, run the sql query: {{select <operator/predicate>
(operand, NULL)}} then assert that the resulting relNode is a
{{LogicalValues(tuples=[[\{ null }]])}} rather than anything else.
{quote}I'd like to remove Strong.MAP populate it from information in the
operators. Similarly, move NullPolicy out of RexImpTable and onto the operators.
{quote} * That sounds interesting. Can you please elaborate on how you plan on
doing this? Are we going to remove both maps and then have one internal method
in each operator/predicate that returns either the policy? What are the
benefits for this approach?
Also, please correct me if I'm wrong, but this change will involve only relying
on either `{{{}NullPolicy{}}}` or `{{{}Strong.Policy`{}}} and not both,
correct? (I.e. removing one of those enums)
Once we agree on these details, I can work on those changes.
was (Author: JIRAUSER307817):
{quote}Now I am worrying that there are many more functions that have the same
problem - ATAN2, for example - and how we could detect and fix these functions
efficiently.
{quote} * [~julianhyde] I agree with you that there might be many more
functions that have the same problem, +i.e.+ having wrong {{{}Strong }}policy
assignment, though their null policy seems to be correct in
`\{{{}RexImpTable{}}}`.
The problem is that, like [~mbudiu] mentioned, we are not using the values in
{{RexImpTable}} in order to simplify {{{}RexNodes{}}}, which leads to operators
like {{*POWER(base, exp)*}} slipping through those simplifications. My question
in this case would be: What are the differences between {{Strong.Policy}} and
{{NullPolicy }}in Apache Calcite? Is it that the {{Strong.Policy}} is used
strictly for {*}predicates{*}, while \{{NullPolicy }}is used for
{*}SqlOperators{*}? Can't we unify their respective definitions somehow since
they overlap in functionality, no?
{quote}My question to you is: What evidence did you see that made you realize
that POWER should be ANY, not AS_IS? What is the test case that passes only
after this bug is fixed? I want to apply that test case automatically to all
functions.
{quote} * The test case that only passes after this bug is fixed is when I
convert the following SQL query to a RelNode and then assert on the output
RelNode. This is what I get:
!image-2024-11-26-11-11-18-721.png!
* With regards to automating this test case to apply to all functions, maybe
we can iterate over all the preciates / operators that we deem to have a
{{Strong.Policy.ANY}} value, run the sql query: {{select <operator/predicate>
(operand, NULL)}} then assert that the resulting relNode is a
{{LogicalValues(tuples=[[\{ null }]])}} rather than anything else.
{quote}I'd like to remove Strong.MAP populate it from information in the
operators. Similarly, move NullPolicy out of RexImpTable and onto the operators.
{quote} * That sounds interesting. Can you please elaborate on how you plan on
doing this? Are we going to remove both maps and then have one internal method
in each operator/predicate that returns either the policy? What are the
benefits for this approach?
Also, please correct me if I'm wrong, but this change will involve only relying
on either `{{{}NullPolicy{}}}` or `{{{}Strong.Policy`{}}} and not both,
correct? (I.e. removing one of those enums)
Once we agree on these details, I can work on those changes.
> Strong Policy for the `SqlStdOperatorTable.POWER` is wrongly assigned `AS_IS`
> when it should be `ANY`
> -----------------------------------------------------------------------------------------------------
>
> Key: CALCITE-6702
> URL: https://issues.apache.org/jira/browse/CALCITE-6702
> Project: Calcite
> Issue Type: Bug
> Reporter: Dhia Eddine Nini
> Assignee: Dhia Eddine Nini
> Priority: Major
> Labels: pull-request-available
> Attachments: image-2024-11-20-17-14-28-261.png,
> image-2024-11-22-12-45-23-068.png, image-2024-11-26-11-11-18-721.png
>
>
> +Context:+
> In standard SQL (such as MySQL and PostgreSQL), running the following queries:
> {code:java}
> SELECT POWER(NULL, 2) AS "ALIAS1";
> SELECT POWER(2, NULL) AS "ALIAS2";{code}
> Returns for both queries:
> {code:java}
> NULL
> {code}
> This means that the proper null policy for the expression is `ANY` instead of
> `AS_IS`, since this expression is null {*}if and only if at least one of its
> arguments is null{*}.
> !image-2024-11-20-17-14-28-261.png!
> The `SqlKind` of the the POWER function is `SqlKind.OTHER_FUNCTION`
> The fix should be relatively simple.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
