From 4e26a2d6a6aee477b990380155d014a33fef5a60 Mon Sep 17 00:00:00 2001 From: Jerome Haltom Date: Sat, 12 Sep 2026 10:27:34 -0500 Subject: [PATCH 1/3] [CALCITE-7778] JDBC adapter for MSSQL generates % for MOD without preserving grouping, giving wrong results --- .../calcite/sql/dialect/MssqlSqlDialect.java | 7 ++--- .../calcite/util/RelToSqlConverterUtil.java | 31 +++++++++++++++++++ .../rel/rel2sql/RelToSqlConverterTest.java | 16 ++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java index 964fa03f8f91..5a8672ae58df 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java @@ -35,8 +35,6 @@ import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; -import org.apache.calcite.sql.SqlOperator; -import org.apache.calcite.sql.SqlSyntax; import org.apache.calcite.sql.SqlUtil; import org.apache.calcite.sql.SqlWriter; import org.apache.calcite.sql.fun.SqlLibraryOperators; @@ -49,6 +47,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import static org.apache.calcite.util.RelToSqlConverterUtil.unparseBoolLiteralToCondition; +import static org.apache.calcite.util.RelToSqlConverterUtil.unparseWithOperator; import static java.util.Objects.requireNonNull; @@ -222,8 +221,8 @@ private static SqlNode createDatetimeCastSpec(String typeAlias, RelDataType type unparseFloor(writer, call); break; case MOD: - SqlOperator op = SqlStdOperatorTable.PERCENT_REMAINDER; - SqlSyntax.BINARY.unparse(writer, op, call, leftPrec, rightPrec); + unparseWithOperator(writer, SqlStdOperatorTable.PERCENT_REMAINDER, call, + leftPrec, rightPrec); break; case SAFE_CAST: // MSSQL uses TRY_CAST instead of SAFE_CAST (BigQuery) diff --git a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java index db417179afc3..08c7b4a74ec0 100644 --- a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java +++ b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java @@ -31,7 +31,9 @@ import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlMapTypeNameSpec; import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.SqlSpecialOperator; +import org.apache.calcite.sql.SqlSyntax; import org.apache.calcite.sql.SqlTypeNameSpec; import org.apache.calcite.sql.SqlWriter; import org.apache.calcite.sql.fun.SqlStdOperatorTable; @@ -339,6 +341,35 @@ public static void unparseBoolLiteralToCondition(SqlWriter writer, boolean value writer.endList(frame); } + /** + * Writes a two-operand call with an operator other than its own, + * parenthesized as that operator requires. + * + *

{@link SqlCall#unparse} chooses the parentheses from the call's own + * operator, before the dialect is consulted. Writing the call with an + * operator that binds less tightly therefore drops parentheses that were + * holding the grouping, and the target system regroups the expression. Where + * the call arrived already parenthesized both precedences are zero and + * nothing further is written. + * + * @param writer current SqlWriter object + * @param operator operator to write the call with + * @param call call to write + * @param leftPrec left precedence the call was given + * @param rightPrec right precedence the call was given + */ + public static void unparseWithOperator(SqlWriter writer, SqlOperator operator, + SqlCall call, int leftPrec, int rightPrec) { + if (leftPrec > operator.getLeftPrec() + || (operator.getRightPrec() <= rightPrec && rightPrec != 0)) { + final SqlWriter.Frame frame = writer.startList("(", ")"); + SqlSyntax.BINARY.unparse(writer, operator, call, 0, 0); + writer.endList(frame); + } else { + SqlSyntax.BINARY.unparse(writer, operator, call, leftPrec, rightPrec); + } + } + /** * Transformation Map type from {@code MAP} to {@code Map(VARCHAR,VARCHAR)}. */ diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 877a1e99ee62..22046bd8a799 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -11820,6 +11820,22 @@ private void checkLiteral2(String expression, String expected) { sql(query).dialect(MssqlSqlDialect.DEFAULT).ok(mssqlExpected); } + /** Test case for + * [CALCITE-7778] + * JDBC adapter for MSSQL generates % for MOD without preserving grouping, + * giving wrong results. */ + @Test void testModFunctionGroupingForMSSQL() { + final String from = "\nFROM (VALUES (0)) AS [t] ([ZERO])"; + sql("select 100 / mod(11, 3)").dialect(MssqlSqlDialect.DEFAULT) + .ok("SELECT 100 / (11 % 3)" + from); + sql("select 100 * mod(11, 3)").dialect(MssqlSqlDialect.DEFAULT) + .ok("SELECT 100 * (11 % 3)" + from); + sql("select mod(100, mod(11, 3))").dialect(MssqlSqlDialect.DEFAULT) + .ok("SELECT 100 % (11 % 3)" + from); + // % already binds more tightly than -, so no parentheses are needed + sql("select 100 - mod(11, 3)").dialect(MssqlSqlDialect.DEFAULT) + .ok("SELECT 100 - 11 % 3" + from); + } /** Test case for * [CALCITE-6655] From ddd9f4b485a0178293fa886e25c2b9921cf2cf1f Mon Sep 17 00:00:00 2001 From: Jerome Haltom Date: Sat, 12 Sep 2026 10:47:27 -0500 Subject: [PATCH 2/3] Shorten the javadoc on unparseWithOperator --- .../apache/calcite/util/RelToSqlConverterUtil.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java index 08c7b4a74ec0..114e8fc00d64 100644 --- a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java +++ b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java @@ -346,17 +346,8 @@ public static void unparseBoolLiteralToCondition(SqlWriter writer, boolean value * parenthesized as that operator requires. * *

{@link SqlCall#unparse} chooses the parentheses from the call's own - * operator, before the dialect is consulted. Writing the call with an - * operator that binds less tightly therefore drops parentheses that were - * holding the grouping, and the target system regroups the expression. Where - * the call arrived already parenthesized both precedences are zero and - * nothing further is written. - * - * @param writer current SqlWriter object - * @param operator operator to write the call with - * @param call call to write - * @param leftPrec left precedence the call was given - * @param rightPrec right precedence the call was given + * operator before the dialect is consulted, so an operator that binds less + * tightly needs them added here. */ public static void unparseWithOperator(SqlWriter writer, SqlOperator operator, SqlCall call, int leftPrec, int rightPrec) { From 0066090f62c9f0d1deafa92d5821fdf9a5ff210c Mon Sep 17 00:00:00 2001 From: Jerome Haltom Date: Mon, 14 Sep 2026 14:59:16 -0500 Subject: [PATCH 3/3] Rename unparseWithOperator to unparseWithBinaryOperator --- .../java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java | 4 ++-- .../java/org/apache/calcite/util/RelToSqlConverterUtil.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java index 5a8672ae58df..979ed20cb111 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java @@ -47,7 +47,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import static org.apache.calcite.util.RelToSqlConverterUtil.unparseBoolLiteralToCondition; -import static org.apache.calcite.util.RelToSqlConverterUtil.unparseWithOperator; +import static org.apache.calcite.util.RelToSqlConverterUtil.unparseWithBinaryOperator; import static java.util.Objects.requireNonNull; @@ -221,7 +221,7 @@ private static SqlNode createDatetimeCastSpec(String typeAlias, RelDataType type unparseFloor(writer, call); break; case MOD: - unparseWithOperator(writer, SqlStdOperatorTable.PERCENT_REMAINDER, call, + unparseWithBinaryOperator(writer, SqlStdOperatorTable.PERCENT_REMAINDER, call, leftPrec, rightPrec); break; case SAFE_CAST: diff --git a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java index 114e8fc00d64..9041dbecd163 100644 --- a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java +++ b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java @@ -349,7 +349,7 @@ public static void unparseBoolLiteralToCondition(SqlWriter writer, boolean value * operator before the dialect is consulted, so an operator that binds less * tightly needs them added here. */ - public static void unparseWithOperator(SqlWriter writer, SqlOperator operator, + public static void unparseWithBinaryOperator(SqlWriter writer, SqlOperator operator, SqlCall call, int leftPrec, int rightPrec) { if (leftPrec > operator.getLeftPrec() || (operator.getRightPrec() <= rightPrec && rightPrec != 0)) {