[CALCITE-6702] Strong Policy for the POWER Function is wrong

[ninidhiaeddine]

CALCITE-6702: Strong Policy for the SqlStdOperatorTable.POWER Function is wrongly assigned AS_IS when it should be ANY.

Context:
In various SQL db implementations, the POWER(a, b) function returns NULL when either or both operands are null.

SELECT POWER(NULL, 2) AS "ALIAS1"; -- => Returns NULL
SELECT POWER(2, NULL) AS "ALIAS2"; -- => Returns NULL
SELECT POWER(NULL ,NULL) AS "ALIAS3"; -- => Returns NULL

These are all the cases I could think of:

This means that the correct null policy for the POWER function is ANY instead of AS_IS, since this expression is NULL if and only if at least one of its arguments is NULL.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[NobiGo]

@ninidhiaeddine We need a unit test for this change.

[ninidhiaeddine]

@ninidhiaeddine We need a unit test for this change.

I was thinking to add a unit test for this change, but I didn't find any existing unit tests that are checking the returned Strong.Policy for a given SqlOperator.

The only unit tests that I found to be remotely related are: SqlOperatorTest#testPowerFun() and SqlOperatorTest#testPowerDecimalFunc() which check that POWER(2, NULL) and POWER(NULL, 2) both return NULL. However, none of these methods make any call to Strong.policy(arg).

Any ideas on what to test here exactly? Please let me know.

[caicancai]

@ninidhiaeddine We need a unit test for this change.

I was thinking to add a unit test for this change, but I didn't find any existing unit tests that are checking the returned Strong.Policy for a given SqlOperator.

The only unit tests that I found to be remotely related are: SqlOperatorTest#testPowerFun() and SqlOperatorTest#testPowerDecimalFunc() which check that POWER(2, NULL) and POWER(NULL, 2) both return NULL. However, none of these methods make any call to Strong.policy(arg).

Any ideas on what to test here exactly? Please let me know.

@ninidhiaeddine Hi. You can add tests in sqloperatortest

[ninidhiaeddine]

@ninidhiaeddine Hi. You can add tests in sqloperatortest

Could you please clarify?
The only relevant check I can do is Assert.assertEquals(Strong.Policy.ANY, Strong.policy(SqlStdOperatorTable.POWER) but it would be out of context as none of the existing test methods in this class currently validate the Strong.Policy of the SqlOperators. What do you think?

[mihaibudiu]

The Strong class is used during optimizations.
The JavaDoc for Strong says "Utilities for strong predicates."
However, POWER is not a predicate.
So I don't know whether this change has any effect.
It is not easy to audit all the places where Strong is used in the codebase to determine whether it will have any effect on any program. Most places I could find only simplify predicates.
I don't know if this has any effect on the *_REDUCE_EXPRESSIONS set of optimization rules, where perhaps POWER with a constant null argument could be simplified to NULL at compile-time.
Do you have an example which is affected by this issue?

[mihaibudiu]

BTW: there is another class NullPolicy which seems to overlap in functionality with Strong. That class is used during code-generation for the Enum interpreter.

[mihaibudiu]

The only place where the Policy is actually used is in RexSimplify, where it is used to check whether some expressions are always null. So perhaps you can can write a SqlToRelConverter test that checks to see whether the call to POWER(null, x) is replaced in the program with a constant NULL.

[mihaibudiu]

Since Strong provides only a conservative approximation, AS_IS is not wrong, it may be just a missed optimization opportunity.

[ninidhiaeddine]

The Strong class is used during optimizations. The JavaDoc for Strong says "Utilities for strong predicates." However, POWER is not a predicate. So I don't know whether this change has any effect. It is not easy to audit all the places where Strong is used in the codebase to determine whether it will have any effect on any program. Most places I could find only simplify predicates. I don't know if this has any effect on the *_REDUCE_EXPRESSIONS set of optimization rules, where perhaps POWER with a constant null argument could be simplified to NULL at compile-time. Do you have an example which is affected by this issue?

Thank you for your reply @mihaibudiu . Let's move the discussion to the Jira issue if you don't mind, since @julianhyde is probably not seeing the discussion here. I wrote my reply there.
Once we agree on the changes, we will continue here

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.