From 9400c180706e0707b0ad99d11d9134392bc42706 Mon Sep 17 00:00:00 2001 From: nobigo Date: Tue, 10 Sep 2024 19:07:26 +0800 Subject: [PATCH 1/5] [CALCITE-6566] JDBC adapter should generate PI function with parentheses When unparse PI function without parentheses --- .../apache/calcite/sql/SqlBasicFunction.java | 11 +++++++++ .../java/org/apache/calcite/sql/SqlKind.java | 3 +++ .../sql/dialect/ClickHouseSqlDialect.java | 4 ++++ .../calcite/sql/dialect/HiveSqlDialect.java | 3 +++ .../calcite/sql/dialect/MssqlSqlDialect.java | 5 +++- .../calcite/sql/dialect/MysqlSqlDialect.java | 7 +++--- .../sql/dialect/PostgresqlSqlDialect.java | 4 ++++ .../calcite/sql/dialect/PrestoSqlDialect.java | 3 +++ .../calcite/sql/dialect/SparkSqlDialect.java | 4 ++++ .../calcite/sql/fun/SqlStdOperatorTable.java | 2 +- .../calcite/util/RelToSqlConverterUtil.java | 21 +++++++++++++++++ .../rel/rel2sql/RelToSqlConverterTest.java | 23 +++++++++++++++++++ 12 files changed, 85 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/SqlBasicFunction.java b/core/src/main/java/org/apache/calcite/sql/SqlBasicFunction.java index 05c1f80c0b6..c60aebede69 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlBasicFunction.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlBasicFunction.java @@ -107,6 +107,17 @@ public static SqlBasicFunction create(String name, SqlKind kind, SqlFunctionCategory.SYSTEM, call -> SqlMonotonicity.NOT_MONOTONIC, false); } + /** Creates a {@code SqlBasicFunction} whose name is the same as its kind. */ + public static SqlBasicFunction create(SqlKind kind, + SqlReturnTypeInference returnTypeInference, + SqlOperandTypeChecker operandTypeChecker, + SqlFunctionCategory category) { + return new SqlBasicFunction(kind.name(), kind, + SqlSyntax.FUNCTION, true, returnTypeInference, null, + OperandHandlers.DEFAULT, operandTypeChecker, 0, + category, call -> SqlMonotonicity.NOT_MONOTONIC, false); + } + /** Creates a {@code SqlBasicFunction} whose name is the same as its kind * and whose category {@link SqlFunctionCategory#SYSTEM}. */ public static SqlBasicFunction create(SqlKind kind, diff --git a/core/src/main/java/org/apache/calcite/sql/SqlKind.java b/core/src/main/java/org/apache/calcite/sql/SqlKind.java index 70094d1b49d..62088f31407 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlKind.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlKind.java @@ -731,6 +731,9 @@ public enum SqlKind { /** {@code CEIL} function. */ CEIL, + /** ${code PI} function. */ + PI, + /** {@code TRIM} function. */ TRIM, diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java index 9fc3618924b..5e43267ccb8 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java @@ -33,6 +33,7 @@ import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.BasicSqlType; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.util.RelToSqlConverterUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -203,6 +204,9 @@ private static SqlDataTypeSpec createSqlDataTypeSpecByName(String typeAlias, call.operand(1).unparse(writer, 0, 0); writer.endList(frame); break; + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java index bf40da483e7..ac1bbcadf60 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java @@ -109,6 +109,9 @@ public HiveSqlDialect(Context context) { case TRIM: RelToSqlConverterUtil.unparseHiveTrim(writer, call, leftPrec, rightPrec); break; + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } 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 6f32a33a4a9..c7ebb668eb3 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 @@ -40,6 +40,7 @@ import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.util.RelToSqlConverterUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -172,7 +173,9 @@ public MssqlSqlDialect(Context context) { } unparseFloor(writer, call); break; - + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java index fc2a7a1b054..1aa27d8993b 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java @@ -48,6 +48,7 @@ import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.util.RelToSqlConverterUtil; import com.google.common.collect.ImmutableList; @@ -242,7 +243,6 @@ public MysqlSqlDialect(Context context) { unparseFloor(writer, call); break; - case WITHIN_GROUP: final List operands = call.getOperandList(); if (operands.size() <= 0 || operands.get(0).getKind() != SqlKind.LISTAGG) { @@ -252,11 +252,12 @@ public MysqlSqlDialect(Context context) { unparseListAggCall(writer, (SqlCall) operands.get(0), operands.size() == 2 ? operands.get(1) : null, leftPrec, rightPrec); break; - case LISTAGG: unparseListAggCall(writer, call, null, leftPrec, rightPrec); break; - + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java index 627b3c79a41..c2eba8aac98 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java @@ -38,6 +38,7 @@ import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.util.RelToSqlConverterUtil; import com.google.common.collect.ImmutableList; @@ -177,6 +178,9 @@ public PostgresqlSqlDialect(Context context) { timeUnitNode.getParserPosition()); SqlFloorFunction.unparseDatetimeFunction(writer, call2, "DATE_TRUNC", false); break; + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java index 4b6c7c3292d..961870cb464 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java @@ -145,6 +145,9 @@ private static void unparseUsingLimit(SqlWriter writer, @Nullable SqlNode offset case MAP_VALUE_CONSTRUCTOR: unparseMapValue(writer, call, leftPrec, rightPrec); break; + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: // Current impl is same with Postgresql. PostgresqlSqlDialect.DEFAULT.unparseCall(writer, call, leftPrec, rightPrec); diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java index c2daf0ab677..c8e0254b069 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java @@ -34,6 +34,7 @@ import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.BasicSqlType; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.util.RelToSqlConverterUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -139,6 +140,9 @@ public SparkSqlDialect(SqlDialect.Context context) { case POSITION: SqlUtil.unparseFunctionSyntax(SqlStdOperatorTable.POSITION, writer, call, false); break; + case PI: + RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java index 0254e2a219b..0ab0a240b16 100644 --- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java +++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java @@ -1815,7 +1815,7 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable { /** The {@code PI} function. */ public static final SqlFunction PI = - SqlBasicFunction.create("PI", ReturnTypes.DOUBLE, OperandTypes.NILADIC, + SqlBasicFunction.create(SqlKind.PI, ReturnTypes.DOUBLE, OperandTypes.NILADIC, SqlFunctionCategory.NUMERIC) .withSyntax(SqlSyntax.FUNCTION_ID); 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 6b510e41417..35373d7b508 100644 --- a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java +++ b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java @@ -93,6 +93,27 @@ public static void unparseSparkArrayAndMap(SqlWriter writer, writer.endList(frame); } + /** + * Unparses PI function without parentheses. + * + *

For example : + * + *

+   * SELECT PI → SELECT PI()
+   * 
+ * + * @param writer writer + * @param call the call + */ + public static void unparsePI( + SqlWriter writer, + SqlCall call, + int leftPrec, + int rightPrec) { + final SqlWriter.Frame trimFrame = writer.startFunCall(call.getKind().name()); + writer.endFunCall(trimFrame); + } + /** * Unparses TRIM function with value as space. * 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 d855bb96c38..5e188c19f86 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 @@ -357,6 +357,29 @@ private static String toSql(RelNode root, SqlDialect dialect, .withStarRocks().ok(expectedStarRocks); } + /** Test case for [CALCITE-6566] + * JDBC adapter should generate PI function with parentheses + * When unparse PI function without parentheses. */ + @Test void testPiFunction() { + String query = "select PI"; + final String expected = "SELECT PI AS \"PI\"\nFROM (VALUES (0)) AS \"t\" (\"ZERO\")"; + final String expectedHive = "SELECT PI() `PI`"; + final String expectedSpark = "SELECT PI() `PI`\nFROM (VALUES (0)) `t` (`ZERO`)"; + final String expectedMssql = "SELECT PI() AS [PI]\nFROM (VALUES (0)) AS [t] ([ZERO])"; + final String expectedMysql = "SELECT PI() AS `PI`"; + final String expectedClickHouse = "SELECT PI() AS `PI`"; + final String expectedPresto = "SELECT PI() AS \"PI\"\nFROM (VALUES (0)) AS \"t\" (\"ZERO\")"; + final String expectedOracle = "SELECT PI \"PI\"\nFROM \"DUAL\""; + sql(query).ok(expected) + .withHive().ok(expectedHive) + .withSpark().ok(expectedSpark) + .withMssql().ok(expectedMssql) + .withMysql().ok(expectedMysql) + .withClickHouse().ok(expectedClickHouse) + .withPresto().ok(expectedPresto) + .withOracle().ok(expectedOracle); + } + @Test void testPivotToSqlFromProductTable() { String query = "select * from (\n" + " select \"shelf_width\", \"net_weight\", \"product_id\"\n" From ae79ae66af0549b1bdfd635c16ff3d21ebae0ef5 Mon Sep 17 00:00:00 2001 From: nobigo Date: Thu, 12 Sep 2024 06:09:08 +0800 Subject: [PATCH 2/5] [CALCITE-6566] Code Review --- .../org/apache/calcite/sql/SqlDialect.java | 4 ++++ .../sql/dialect/CalciteSqlDialect.java | 13 ++++++++++++ .../sql/dialect/ClickHouseSqlDialect.java | 4 ---- .../calcite/sql/dialect/HiveSqlDialect.java | 3 --- .../calcite/sql/dialect/MssqlSqlDialect.java | 5 +---- .../calcite/sql/dialect/MysqlSqlDialect.java | 7 +++---- .../calcite/sql/dialect/OracleSqlDialect.java | 4 +++- .../sql/dialect/PostgresqlSqlDialect.java | 4 ---- .../calcite/sql/dialect/PrestoSqlDialect.java | 3 --- .../calcite/sql/dialect/SparkSqlDialect.java | 4 ---- .../calcite/util/RelToSqlConverterUtil.java | 6 ++---- .../rel/rel2sql/RelToSqlConverterTest.java | 21 ++++++++++++------- 12 files changed, 39 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java index 9ebc4c383c4..c84de56513c 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java @@ -40,6 +40,7 @@ import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.sql.validate.SqlConformance; import org.apache.calcite.sql.validate.SqlConformanceEnum; +import org.apache.calcite.util.RelToSqlConverterUtil; import org.apache.calcite.util.format.FormatModel; import org.apache.calcite.util.format.FormatModels; @@ -446,6 +447,9 @@ public void unparseCall(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) { SqlOperator operator = call.getOperator(); switch (call.getKind()) { + case PI: + RelToSqlConverterUtil.unparsePiFunction(writer, call); + break; case ROW: // Remove the ROW keyword if the dialect does not allow that. if (!getConformance().allowExplicitRowValueConstructor()) { diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java index f9c381c9217..a0dc044519a 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java @@ -16,7 +16,9 @@ */ package org.apache.calcite.sql.dialect; +import org.apache.calcite.sql.SqlCall; import org.apache.calcite.sql.SqlDialect; +import org.apache.calcite.sql.SqlWriter; /** * A SqlDialect implementation that produces SQL that can be parsed @@ -39,4 +41,15 @@ public class CalciteSqlDialect extends SqlDialect { public CalciteSqlDialect(Context context) { super(context); } + + @Override public void unparseCall(SqlWriter writer, SqlCall call, + int leftPrec, int rightPrec) { + switch (call.getKind()) { + case PI: + call.getOperator().unparse(writer, call, leftPrec, rightPrec); + break; + default: + super.unparseCall(writer, call, leftPrec, rightPrec); + } + } } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java index 5e43267ccb8..9fc3618924b 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/ClickHouseSqlDialect.java @@ -33,7 +33,6 @@ import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.BasicSqlType; import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.util.RelToSqlConverterUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -204,9 +203,6 @@ private static SqlDataTypeSpec createSqlDataTypeSpecByName(String typeAlias, call.operand(1).unparse(writer, 0, 0); writer.endList(frame); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java index ac1bbcadf60..bf40da483e7 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java @@ -109,9 +109,6 @@ public HiveSqlDialect(Context context) { case TRIM: RelToSqlConverterUtil.unparseHiveTrim(writer, call, leftPrec, rightPrec); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } 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 c7ebb668eb3..6f32a33a4a9 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 @@ -40,7 +40,6 @@ import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.util.RelToSqlConverterUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -173,9 +172,7 @@ public MssqlSqlDialect(Context context) { } unparseFloor(writer, call); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; + default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java index 1aa27d8993b..fc2a7a1b054 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/MysqlSqlDialect.java @@ -48,7 +48,6 @@ import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.util.RelToSqlConverterUtil; import com.google.common.collect.ImmutableList; @@ -243,6 +242,7 @@ public MysqlSqlDialect(Context context) { unparseFloor(writer, call); break; + case WITHIN_GROUP: final List operands = call.getOperandList(); if (operands.size() <= 0 || operands.get(0).getKind() != SqlKind.LISTAGG) { @@ -252,12 +252,11 @@ public MysqlSqlDialect(Context context) { unparseListAggCall(writer, (SqlCall) operands.get(0), operands.size() == 2 ? operands.get(1) : null, leftPrec, rightPrec); break; + case LISTAGG: unparseListAggCall(writer, call, null, leftPrec, rightPrec); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; + default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java index 75ce64286b5..88720379951 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java @@ -204,7 +204,9 @@ public OracleSqlDialect(Context context) { timeUnitNode.getParserPosition()); SqlFloorFunction.unparseDatetimeFunction(writer, call2, "TRUNC", true); break; - + case PI: + call.getOperator().unparse(writer, call, leftPrec, rightPrec); + break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java index c2eba8aac98..627b3c79a41 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java @@ -38,7 +38,6 @@ import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.util.RelToSqlConverterUtil; import com.google.common.collect.ImmutableList; @@ -178,9 +177,6 @@ public PostgresqlSqlDialect(Context context) { timeUnitNode.getParserPosition()); SqlFloorFunction.unparseDatetimeFunction(writer, call2, "DATE_TRUNC", false); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java index 961870cb464..4b6c7c3292d 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/PrestoSqlDialect.java @@ -145,9 +145,6 @@ private static void unparseUsingLimit(SqlWriter writer, @Nullable SqlNode offset case MAP_VALUE_CONSTRUCTOR: unparseMapValue(writer, call, leftPrec, rightPrec); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; default: // Current impl is same with Postgresql. PostgresqlSqlDialect.DEFAULT.unparseCall(writer, call, leftPrec, rightPrec); diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java index c8e0254b069..c2daf0ab677 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/SparkSqlDialect.java @@ -34,7 +34,6 @@ import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.BasicSqlType; import org.apache.calcite.sql.type.SqlTypeName; -import org.apache.calcite.util.RelToSqlConverterUtil; import org.checkerframework.checker.nullness.qual.Nullable; @@ -140,9 +139,6 @@ public SparkSqlDialect(SqlDialect.Context context) { case POSITION: SqlUtil.unparseFunctionSyntax(SqlStdOperatorTable.POSITION, writer, call, false); break; - case PI: - RelToSqlConverterUtil.unparsePI(writer, call, leftPrec, rightPrec); - break; default: super.unparseCall(writer, call, leftPrec, rightPrec); } 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 35373d7b508..69a084561ee 100644 --- a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java +++ b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java @@ -105,11 +105,9 @@ public static void unparseSparkArrayAndMap(SqlWriter writer, * @param writer writer * @param call the call */ - public static void unparsePI( + public static void unparsePiFunction( SqlWriter writer, - SqlCall call, - int leftPrec, - int rightPrec) { + SqlCall call) { final SqlWriter.Frame trimFrame = writer.startFunCall(call.getKind().name()); writer.endFunCall(trimFrame); } 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 5e188c19f86..abbde238d4a 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 @@ -358,19 +358,24 @@ private static String toSql(RelNode root, SqlDialect dialect, } /** Test case for [CALCITE-6566] - * JDBC adapter should generate PI function with parentheses - * When unparse PI function without parentheses. */ + * JDBC adapter should generate PI function with parentheses in most dialects. */ @Test void testPiFunction() { String query = "select PI"; - final String expected = "SELECT PI AS \"PI\"\nFROM (VALUES (0)) AS \"t\" (\"ZERO\")"; + final String expected = "SELECT PI AS \"PI\"\n" + + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")"; final String expectedHive = "SELECT PI() `PI`"; - final String expectedSpark = "SELECT PI() `PI`\nFROM (VALUES (0)) `t` (`ZERO`)"; - final String expectedMssql = "SELECT PI() AS [PI]\nFROM (VALUES (0)) AS [t] ([ZERO])"; + final String expectedSpark = "SELECT PI() `PI`\n" + + "FROM (VALUES (0)) `t` (`ZERO`)"; + final String expectedMssql = "SELECT PI() AS [PI]\n" + + "FROM (VALUES (0)) AS [t] ([ZERO])"; final String expectedMysql = "SELECT PI() AS `PI`"; final String expectedClickHouse = "SELECT PI() AS `PI`"; - final String expectedPresto = "SELECT PI() AS \"PI\"\nFROM (VALUES (0)) AS \"t\" (\"ZERO\")"; - final String expectedOracle = "SELECT PI \"PI\"\nFROM \"DUAL\""; - sql(query).ok(expected) + final String expectedPresto = "SELECT PI() AS \"PI\"\n" + + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")"; + final String expectedOracle = "SELECT PI \"PI\"\n" + + "FROM \"DUAL\""; + sql(query) + .ok(expected) .withHive().ok(expectedHive) .withSpark().ok(expectedSpark) .withMssql().ok(expectedMssql) From 3f9c5f0ecb94d257e637fcad7e4d94a102561022 Mon Sep 17 00:00:00 2001 From: nobigo Date: Thu, 19 Sep 2024 06:49:03 +0800 Subject: [PATCH 3/5] [CALCITE-6566] Support PI() replace PI --- .../org/apache/calcite/sql/SqlDialect.java | 4 -- .../sql/dialect/CalciteSqlDialect.java | 13 ------- .../calcite/sql/dialect/OracleSqlDialect.java | 2 +- .../calcite/sql/fun/SqlStdOperatorTable.java | 4 +- .../calcite/util/RelToSqlConverterUtil.java | 19 ---------- .../rel/rel2sql/RelToSqlConverterTest.java | 37 ++++++++++++++----- 6 files changed, 30 insertions(+), 49 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java index c84de56513c..9ebc4c383c4 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlDialect.java @@ -40,7 +40,6 @@ import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.sql.validate.SqlConformance; import org.apache.calcite.sql.validate.SqlConformanceEnum; -import org.apache.calcite.util.RelToSqlConverterUtil; import org.apache.calcite.util.format.FormatModel; import org.apache.calcite.util.format.FormatModels; @@ -447,9 +446,6 @@ public void unparseCall(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) { SqlOperator operator = call.getOperator(); switch (call.getKind()) { - case PI: - RelToSqlConverterUtil.unparsePiFunction(writer, call); - break; case ROW: // Remove the ROW keyword if the dialect does not allow that. if (!getConformance().allowExplicitRowValueConstructor()) { diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java index a0dc044519a..f9c381c9217 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/CalciteSqlDialect.java @@ -16,9 +16,7 @@ */ package org.apache.calcite.sql.dialect; -import org.apache.calcite.sql.SqlCall; import org.apache.calcite.sql.SqlDialect; -import org.apache.calcite.sql.SqlWriter; /** * A SqlDialect implementation that produces SQL that can be parsed @@ -41,15 +39,4 @@ public class CalciteSqlDialect extends SqlDialect { public CalciteSqlDialect(Context context) { super(context); } - - @Override public void unparseCall(SqlWriter writer, SqlCall call, - int leftPrec, int rightPrec) { - switch (call.getKind()) { - case PI: - call.getOperator().unparse(writer, call, leftPrec, rightPrec); - break; - default: - super.unparseCall(writer, call, leftPrec, rightPrec); - } - } } diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java index 88720379951..231c560a0dc 100644 --- a/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java +++ b/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java @@ -205,7 +205,7 @@ public OracleSqlDialect(Context context) { SqlFloorFunction.unparseDatetimeFunction(writer, call2, "TRUNC", true); break; case PI: - call.getOperator().unparse(writer, call, leftPrec, rightPrec); + writer.sep(call.getOperator().getName()); break; default: super.unparseCall(writer, call, leftPrec, rightPrec); diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java index 0ab0a240b16..d0b694165ba 100644 --- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java +++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java @@ -50,7 +50,6 @@ import org.apache.calcite.sql.SqlSetOperator; import org.apache.calcite.sql.SqlSetSemanticsTableOperator; import org.apache.calcite.sql.SqlSpecialOperator; -import org.apache.calcite.sql.SqlSyntax; import org.apache.calcite.sql.SqlTumbleTableFunction; import org.apache.calcite.sql.SqlUnnestOperator; import org.apache.calcite.sql.SqlUtil; @@ -1816,8 +1815,7 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable { /** The {@code PI} function. */ public static final SqlFunction PI = SqlBasicFunction.create(SqlKind.PI, ReturnTypes.DOUBLE, OperandTypes.NILADIC, - SqlFunctionCategory.NUMERIC) - .withSyntax(SqlSyntax.FUNCTION_ID); + SqlFunctionCategory.NUMERIC); /** {@code FIRST} function to be used within {@code MATCH_RECOGNIZE}. */ public static final SqlFunction FIRST = 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 69a084561ee..6b510e41417 100644 --- a/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java +++ b/core/src/main/java/org/apache/calcite/util/RelToSqlConverterUtil.java @@ -93,25 +93,6 @@ public static void unparseSparkArrayAndMap(SqlWriter writer, writer.endList(frame); } - /** - * Unparses PI function without parentheses. - * - *

For example : - * - *

-   * SELECT PI → SELECT PI()
-   * 
- * - * @param writer writer - * @param call the call - */ - public static void unparsePiFunction( - SqlWriter writer, - SqlCall call) { - final SqlWriter.Frame trimFrame = writer.startFunCall(call.getKind().name()); - writer.endFunCall(trimFrame); - } - /** * Unparses TRIM function with value as space. * 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 abbde238d4a..9dfddb0ad11 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 @@ -360,19 +360,19 @@ private static String toSql(RelNode root, SqlDialect dialect, /** Test case for [CALCITE-6566] * JDBC adapter should generate PI function with parentheses in most dialects. */ @Test void testPiFunction() { - String query = "select PI"; - final String expected = "SELECT PI AS \"PI\"\n" + String query = "select PI()"; + final String expected = "SELECT PI()\n" + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")"; - final String expectedHive = "SELECT PI() `PI`"; - final String expectedSpark = "SELECT PI() `PI`\n" + final String expectedHive = "SELECT PI()"; + final String expectedSpark = "SELECT PI()\n" + "FROM (VALUES (0)) `t` (`ZERO`)"; - final String expectedMssql = "SELECT PI() AS [PI]\n" + final String expectedMssql = "SELECT PI()\n" + "FROM (VALUES (0)) AS [t] ([ZERO])"; - final String expectedMysql = "SELECT PI() AS `PI`"; - final String expectedClickHouse = "SELECT PI() AS `PI`"; - final String expectedPresto = "SELECT PI() AS \"PI\"\n" + final String expectedMysql = "SELECT PI()"; + final String expectedClickHouse = "SELECT PI()"; + final String expectedPresto = "SELECT PI()\n" + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")"; - final String expectedOracle = "SELECT PI \"PI\"\n" + final String expectedOracle = "SELECT PI\n" + "FROM \"DUAL\""; sql(query) .ok(expected) @@ -385,6 +385,25 @@ private static String toSql(RelNode root, SqlDialect dialect, .withOracle().ok(expectedOracle); } + @Test void testNiladicCurrentDateFunction() { + String query = "select CURRENT_DATE"; + final String expected = "SELECT CURRENT_DATE AS \"CURRENT_DATE\"\n" + + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")"; + final String expectedPostgresql = "SELECT CURRENT_DATE AS \"CURRENT_DATE\"\n" + + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")"; + final String expectedSpark = "SELECT CURRENT_DATE `CURRENT_DATE`\n" + + "FROM (VALUES (0)) `t` (`ZERO`)"; + final String expectedMysql = "SELECT CURRENT_DATE AS `CURRENT_DATE`"; + final String expectedOracle = "SELECT CURRENT_DATE \"CURRENT_DATE\"\n" + + "FROM \"DUAL\""; + sql(query) + .ok(expected) + .withPostgresql().ok(expectedPostgresql) + .withSpark().ok(expectedSpark) + .withMysql().ok(expectedMysql) + .withOracle().ok(expectedOracle); + } + @Test void testPivotToSqlFromProductTable() { String query = "select * from (\n" + " select \"shelf_width\", \"net_weight\", \"product_id\"\n" From 08cb0850193230beae5e3a337bd27d6679274276 Mon Sep 17 00:00:00 2001 From: nobigo Date: Thu, 19 Sep 2024 07:17:20 +0800 Subject: [PATCH 4/5] [CALCITE-6566] Make test pass --- babel/src/test/resources/sql/redshift.iq | 12 ++++++------ core/src/test/resources/sql/misc.iq | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/babel/src/test/resources/sql/redshift.iq b/babel/src/test/resources/sql/redshift.iq index d5d9f255f3a..76687288c76 100755 --- a/babel/src/test/resources/sql/redshift.iq +++ b/babel/src/test/resources/sql/redshift.iq @@ -1476,7 +1476,7 @@ EXPR$0 2 !ok -select -abs(-pi); +select -abs(-pi()); EXPR$0 -3.141592653589793 !ok @@ -1522,7 +1522,7 @@ EXPR$0 -10 !ok -select ceil(pi); +select ceil(pi()); EXPR$0 4.0 !ok @@ -1546,7 +1546,7 @@ EXPR$0 !ok # DEGREES -select degrees(pi); +select degrees(pi()); EXPR$0 180.0 !ok @@ -1629,17 +1629,17 @@ SELECT "RANDOM"() !explain-validated-on calcite # ROUND -select round(pi); +select round(pi()); EXPR$0 3.0 !ok -select round(pi, 2); +select round(pi(), 2); EXPR$0 3.14 !ok -select round(-pi, 2); +select round(-pi(), 2); EXPR$0 -3.14 !ok diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq index 2d61e279507..478075e035c 100644 --- a/core/src/test/resources/sql/misc.iq +++ b/core/src/test/resources/sql/misc.iq @@ -1815,7 +1815,7 @@ from (values 1, 2, 3, 4, 5) as t(i); !ok # PI function -values pi; +values pi(); +-------------------+ | PI | +-------------------+ From bdeaae86c535364d3e78a6faaff6e4d9e258a051 Mon Sep 17 00:00:00 2001 From: nobigo Date: Thu, 19 Sep 2024 07:23:49 +0800 Subject: [PATCH 5/5] [CALCITE-6566] Make test pass --- core/src/test/resources/sql/misc.iq | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/test/resources/sql/misc.iq b/core/src/test/resources/sql/misc.iq index 478075e035c..1dd8e18be9e 100644 --- a/core/src/test/resources/sql/misc.iq +++ b/core/src/test/resources/sql/misc.iq @@ -1817,7 +1817,7 @@ from (values 1, 2, 3, 4, 5) as t(i); # PI function values pi(); +-------------------+ -| PI | +| EXPR$0 | +-------------------+ | 3.141592653589793 | +-------------------+ @@ -1826,7 +1826,7 @@ values pi(); !ok # DEGREES function -values (degrees(pi), degrees(-pi / 2)); +values (degrees(pi()), degrees(-pi() / 2)); +--------+--------+ | EXPR$0 | EXPR$1 | +--------+--------+