[FLINK-39577][table] Reuse Calcite's `COALESCE` to apply simplifications from `RexSimplify` by snuyanzin · Pull Request #28066 · apache/flink

snuyanzin · 2026-04-29T12:43:56Z

What is the purpose of the change

There are multiple simplification in Calcite around COALESCE
the problem is that they are not applicable in Flink because it's custom implementation
Luckily we are in a phase where replacement is not painful
The PR is making that

Brief change log

Use Calcite's COALESCE

Verifying this change

Existing COALESCE tests

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): ( no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
The serializers: (no )
The runtime per-record code paths (performance sensitive): (no )
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no )
The S3 file system connector: (no )

Documentation

Does this pull request introduce a new feature? ( no)
If yes, how is the feature documented? (not applicable)

Was generative AI tooling used to co-author this PR?

Yes (please specify the tool below)

snuyanzin · 2026-04-29T12:44:57Z

+
+    // ~ Methods ----------------------------------------------------------------
+
+    // ----- FLINK MODIFICATION BEGIN -----


In calcite's properties there is a flag to turn it off
the problem with that flag is that it turns off for all functions, not only for coalesce

Can you say more about what this modification does?

it's a standard way of marking places in classes copied from Calcite what exactly area is changed in this classes

Sorry, I do understand that we are overriding / changing something from Calcite.

I am trying to understand what we are changing about the Calcite class.

see here https://github.com/apache/calcite/blob/b008df9a71cea5da44674c07e8599a0836ad1aad/core/src/main/java/org/apache/calcite/sql/fun/SqlCoalesceFunction.java#L75-L119

by default Calcite rewrites COALESCE with CASE ... WHEN and we don't want that

snuyanzin · 2026-04-29T12:45:53Z

- */
-@Internal
-@Value.Enclosing
-public class RemoveUnreachableCoalesceArgumentsRule


completely no need for this as we rely on Calcite's RexSimplify

snuyanzin · 2026-04-29T12:46:14Z

+        CoerceInputsRule.Config.DEFAULT
+          .withCoerceNames(false)
+          .withConsumerRelClass(classOf[LogicalUnion])
+          .toRule,
        // ensure intersect set operator have the same row type
-        new CoerceInputsRule(classOf[LogicalIntersect], false),
+        CoerceInputsRule.Config.DEFAULT
+          .withCoerceNames(false)
+          .withConsumerRelClass(classOf[LogicalIntersect])
+          .toRule,
        // ensure except set operator have the same row type
-        new CoerceInputsRule(classOf[LogicalMinus], false),
+        CoerceInputsRule.Config.DEFAULT
+          .withCoerceNames(false)
+          .withConsumerRelClass(classOf[LogicalMinus])
+          .toRule,


replace deprecated calls

snuyanzin · 2026-04-29T12:46:37Z

    <Resource name="ast">
      <![CDATA[
-LogicalProject(EXPR$0=[COALESCE($1, _UTF-16LE'-')])
+LogicalProject(EXPR$0=[$1])


Calcite was able to detect that the arg is not null

snuyanzin · 2026-04-29T12:47:02Z

    <Resource name="ast">
      <![CDATA[
-LogicalProject(field2=[$1], transactionId=[COALESCE($0.data.nested.trId)])
+LogicalProject(field2=[$1], transactionId=[CAST($0.data.nested.trId):VARCHAR(2147483647) CHARACTER SET "UTF-16LE"])


there is only one arg, no need for coalesce

flinkbot · 2026-04-29T12:48:27Z

CI report:

aefcfc3 Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

snuyanzin · 2026-04-29T12:59:55Z

    }
  }

+  def generateCoalesce(


this will also fix performance drop for coalesce
mentioned at FLINK-38468

jnh5y · 2026-04-29T15:55:15Z

+        if (hasNulls && nonNullNodes.size() == operands.size()) {
+            return SqlLiteral.createNull(pos);
+        }


I am having trouble understanding this condition.

For each operand, we either set hasNulls or add to the size of the nonNullNodes, right?

Given that, I don't understand how we this could be true. When hasNulls is true, then there's at least one operand which is not added to nonNullNodes, right?

i rewrote this part in proper way

jnh5y · 2026-04-29T17:12:23Z

    <Resource name="ast">
      <![CDATA[
-LogicalProject(EXPR$0=[COALESCE($1, _UTF-16LE'-')])
+LogicalProject(EXPR$0=[$1])


Since RemoveUnreachableCoalesceArgumentsRule.java has been deleted, should this file be renamed? (non-blocking nit)

I'm fin with current name it still checks for unreachable args
do you have a better name to propose?

jnh5y · 2026-04-29T17:15:18Z

+        SqlParserPos pos = call.getParserPosition();
+        List<SqlNode> nodes = new ArrayList<>();
+        for (SqlNode operand : operands) {
+            nodes.add(Objects.requireNonNullElseGet(operand, () -> SqlLiteral.createNull(pos)));


The Calcite version doesn't seem to be checking if an operand is null.

Is some path creating COALESCE(...,null,...) with a Java-null operand?

it does, however it does it for CaseCall

…ons from `RexSimplify`

jnh5y · 2026-04-30T17:49:36Z

+        SqlParserPos pos = call.getParserPosition();
+        List<SqlNode> nodes = new ArrayList<>();
+        for (SqlNode operand : operands) {
+            if (!SqlUtil.isNullLiteral(operand, false)) {


Is it intentional that we are not checking for casts?

Could something interesting happen if someone trying to cast a wider type to null?

jnh5y · 2026-04-30T17:50:39Z

+
+import static java.util.Objects.requireNonNull;
+
+/** The class copied from Calcite in order to turn off COALESCE rewrite with CASE ... WHEN ... */


Do we want to add a note that we are removing the NULLs?

snuyanzin commented Apr 29, 2026

View reviewed changes

jnh5y reviewed Apr 29, 2026

View reviewed changes

github-actions Bot added the community-reviewed PR has been reviewed by the community. label Apr 29, 2026

jnh5y reviewed Apr 29, 2026

View reviewed changes

[FLINK-39577][table] Reuse Calcite's COALESCE to apply simplificati…

aefcfc3

…ons from `RexSimplify`

snuyanzin force-pushed the flink39577 branch from 7d6df58 to aefcfc3 Compare April 29, 2026 19:56

jnh5y reviewed Apr 30, 2026

View reviewed changes


		// ~ Methods ----------------------------------------------------------------

		// ----- FLINK MODIFICATION BEGIN -----


		import static java.util.Objects.requireNonNull;

		/** The class copied from Calcite in order to turn off COALESCE rewrite with CASE ... WHEN ... */

Conversation

snuyanzin commented Apr 29, 2026

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Was generative AI tooling used to co-author this PR?

Uh oh!

snuyanzin Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flinkbot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snuyanzin Apr 29, 2026 •

edited

Loading

snuyanzin Apr 29, 2026 •

edited

Loading

snuyanzin Apr 29, 2026 •

edited

Loading

flinkbot commented Apr 29, 2026 •

edited

Loading