Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.apache.flink.table.planner.functions.bridging.BridgingSqlFunction;
import org.apache.flink.table.planner.plan.utils.FlinkRexUtil;

import org.apache.flink.shaded.guava33.com.google.common.collect.ImmutableList;

import org.apache.calcite.plan.RelOptRuleCall;
import org.apache.calcite.plan.RelRule;
import org.apache.calcite.rel.RelNode;
Expand All @@ -32,9 +34,12 @@
import org.apache.calcite.rel.core.Project;
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
import org.apache.calcite.rex.RexInputRef;
import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexShuttle;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.immutables.value.Value;

import java.util.List;
Expand Down Expand Up @@ -100,6 +105,10 @@ public RexNode visitCall(RexCall call) {
}
}

if (containsFunctionCalls(call)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could you raise a Jira for this with the motivation behind this change and reference the Jira in the title. From the code it looks like this is more than an optimization it is adding function specific code, it would be good to call out the value being added here and/or symptoms/ behaviour/ errors that might be seen prior to this change.

As there is logic added in the change , I suggest adding unit tests for the code you have added.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Thank you

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I have added Jira and unit tests
@davidradl

return convertToCaseWhen(call);
}

// If it's the last argument, or no non-null argument was found, return the original
// call
if (firstNonNullableArgIndex == call.operands.size() - 1
Expand All @@ -121,6 +130,57 @@ private int getFirstNonNullableArgumentIndex(RexCall call) {
}
return -1;
}

private RexNode convertToCaseWhen(RexCall call) {
List<RexNode> operands = call.operands;

if (operands.size() == 1) {
return operands.get(0);
}

RexBuilder rexBuilder = this.rexBuilder;
ImmutableList.Builder<RexNode> caseArgs = ImmutableList.builder();

for (int i = 0; i < operands.size() - 1; i++) {
RexNode operand = operands.get(i);
RexNode isNotNullCheck =
rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, operand);
caseArgs.add(isNotNullCheck);
caseArgs.add(operand);
}

caseArgs.add(operands.get(operands.size() - 1));

return rexBuilder.makeCall(SqlStdOperatorTable.CASE, caseArgs.build());
}

private boolean containsFunctionCalls(RexCall call) {
for (RexNode operand : call.operands) {
if (isFunctionCall(operand)) {
return true;
}
}
return false;
}

private boolean isFunctionCall(RexNode node) {
return node instanceof RexCall
&& !isSimpleCast((RexCall) node)
&& !isFieldReference(node)
&& !isLiteral(node);
}

private boolean isSimpleCast(RexCall call) {
return call.getOperator().getName().equalsIgnoreCase("CAST");
}

private boolean isFieldReference(RexNode node) {
return node instanceof RexInputRef;
}

private boolean isLiteral(RexNode node) {
return node instanceof RexLiteral;
}
}

private static boolean hasCoalesceInvocation(RexNode node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ void testJoinCoalesce() {
"SELECT * FROM T t1 LEFT JOIN T t2 ON COALESCE(t1.f0, '-', t1.f2) = t2.f0");
}

@Test
void testFunctionCoalesce() {
util.verifyRelPlan("SELECT COALESCE(f0, JSON_VALUE('{\"a\": true}', '$.a')) FROM T");
}

@Test
void testMultipleCoalesces() {
util.verifyRelPlan(
Expand Down