rubenada opened a new pull request, #5184: URL: https://github.com/apache/calcite/pull/5184
<!-- Thanks for sending a pull request! Here are some critical tips for you: 1. READ THE GUIDE FIRST: https://calcite.apache.org/develop/#contributing *For significant contributions, please discuss on the dev mailing list or Jira BEFORE coding.* 2. JIRA IS USUALLY MANDATORY: Ensure you have created an issue on the Calcite Jira: https://issues.apache.org/jira/projects/CALCITE/issues *Check existing issues first to avoid duplicates.* *Note: A Jira is NOT required for typos and cosmetic changes (i.e., changes that are neither bugs nor features).* 3. TIMING: Strongly recommended to create the Jira BEFORE you start writing code (e.g., a day or so before posting a PR). This gives others a chance to weigh in on your specification. 4. CRITICAL CONSISTENCY RULE The following three items MUST match exactly in wording and meaning: (A) The Jira Issue Title (B) This Pull Request Title (C) Your Git Commit Message Format: [CALCITE-XXXX] <Description> Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE Guidelines for a good Title: - Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names. - Focus on the specification and user experience, not the implementation. - Make it clear whether it is a bug or a feature. - Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file"). 5. REPRODUCTION: If fixing a bug, please provide a concise SQL example or test case to reproduce the issue for a faster review. 6. TESTING: Ensure `./gradlew build` passes and appropriate tests are added/updated. --> ## Jira Link [CALCITE-7722](https://issues.apache.org/jira/browse/CALCITE-7722) ## Changes Proposed <!-- Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. If possible, please consider writing useful notes for better and faster reviews in your PR. --> RexSimplify.simplifyIsNotNull / simplifyIsNull currently bail out of the whole simplification when the input RexCall is not fully safe (i.e. isSafeExpression(a) == false). This is stricter than necessary for operators with Strong.Policy.ANY, where IS [NOT] NULL(f(x, y, ...)) is semantically equivalent to IS [NOT] NULL(x) OR/AND IS [NOT] NULL(y) OR/AND ... — the operator itself does not need to be evaluated to compute the result. Example (regression for downstream projects such as Hive): Before (≤ 1.34): IS NOT NULL(CAST(key AS DOUBLE) + 1.0) → IS NOT NULL(CAST(key AS DOUBLE)) After (≥ 1.35): IS NOT NULL(CAST(key AS DOUBLE) + 1.0) → (unchanged) The rewrite is dropped because CAST(key AS DOUBLE) + 1.0 is a non-lossless cast wrapped in a +, so isSafeExpression returns false — even though + is Strong.ANY and the distribution is a valid rewrite regardless of the outer call's safety. Proposed fix: In the Strong.Policy.ANY branch, replace the full-tree safety requirement with a shallow safety check on the outer call. Because the branch rewraps the input as IS [NOT] NULL(operand_i) and joins the results with OR/AND, add a per-operand guard to prevent RexCall.isAlwaysTrue()/isAlwaysFalse() from silently collapsing a rewrapped IS [NOT] NULL(op) whose operand is typed non-nullable but not fully safe (which would otherwise erase a throwing subexpression such as 1 / 0). Strong.Policy.NOT_NULL and CUSTOM continue to require full-tree safety, since those branches drop the subtree entirely. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
