From 12892eb633320f02665ca931db72039a09a8ea20 Mon Sep 17 00:00:00 2001 From: bvolpato Date: Sat, 11 Jul 2026 17:04:23 -0400 Subject: [PATCH] fix(isthmus): support variadic concat conversion Nest Substrait concat arguments into binary Calcite calls while preserving operand order. Closes #724. --- .../expression/ExpressionRexConverter.java | 7 +++++ .../isthmus/FunctionConversionTest.java | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java index ea31d484b..b5dd28551 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -521,6 +521,13 @@ public RexNode visit(Expression.ScalarFunctionInvocation expr, Context context) .collect(Collectors.toList()); RelDataType returnType = typeConverter.toCalcite(typeFactory, expr.outputType()); + if (operator == SqlStdOperatorTable.CONCAT && args.size() > 2) { + return args.stream() + .skip(1) + .reduce( + args.get(0), + (left, right) -> rexBuilder.makeCall(returnType, operator, List.of(left, right))); + } return rexBuilder.makeCall(returnType, operator, args); } diff --git a/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java b/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java index 5e4e80ee2..503a72d92 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java @@ -20,6 +20,7 @@ import org.apache.calcite.rex.RexCall; import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.junit.jupiter.api.Test; /** @@ -212,6 +213,31 @@ void concatStringLiteralAndChar() throws Exception { assertProtoPlanRoundrip("select 'brand_'||P_BRAND from PART"); } + @Test + void variadicConcat() { + ScalarFunctionInvocation concat = + sb.scalarFn( + DefaultExtensionCatalog.FUNCTIONS_STRING, + "concat:str", + TypeCreator.REQUIRED.STRING, + Expression.StrLiteral.builder().value("a").build(), + Expression.StrLiteral.builder().value("b").build(), + Expression.StrLiteral.builder().value("c").build()); + + RexCall outer = + assertInstanceOf( + RexCall.class, concat.accept(expressionRexConverter, Context.newContext())); + assertEquals(SqlStdOperatorTable.CONCAT, outer.getOperator()); + assertEquals(2, outer.getOperands().size()); + + RexCall inner = assertInstanceOf(RexCall.class, outer.getOperands().get(0)); + assertEquals(SqlStdOperatorTable.CONCAT, inner.getOperator()); + assertEquals(2, inner.getOperands().size()); + assertEquals("'a':VARCHAR", inner.getOperands().get(0).toString()); + assertEquals("'b':VARCHAR", inner.getOperands().get(1).toString()); + assertEquals("'c':VARCHAR", outer.getOperands().get(1).toString()); + } + @Test void strptimeTime() { Expression.StrLiteral inputString = Expression.StrLiteral.builder().value("12:34:56").build();