This is an automated email from the ASF dual-hosted git repository.
xiong pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new cd0b8da25f [CALCITE-6566] JDBC adapter should generate PI function
with parentheses in most dialects
cd0b8da25f is described below
commit cd0b8da25f18f258cf9b5d469d3377d17d5ed0dd
Author: Xiong Duan <[email protected]>
AuthorDate: Wed Nov 20 19:30:59 2024 +0800
[CALCITE-6566] JDBC adapter should generate PI function with parentheses in
most dialects
---
.../java/org/apache/calcite/sql/SqlSyntax.java | 17 +++++
.../main/java/org/apache/calcite/sql/SqlUtil.java | 4 +-
.../calcite/sql/dialect/OracleSqlDialect.java | 73 ++++++++++++----------
.../calcite/sql/fun/SqlStdOperatorTable.java | 2 +-
.../sql/validate/SqlAbstractConformance.java | 4 ++
.../calcite/sql/validate/SqlConformance.java | 24 +++++++
.../calcite/sql/validate/SqlConformanceEnum.java | 13 ++++
.../sql/validate/SqlDelegatingConformance.java | 3 +
.../calcite/sql/validate/SqlValidatorImpl.java | 17 +++--
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 73 ++++++++++++++++++++++
core/src/test/resources/sql/misc.iq | 52 +++++++++++++++
.../org/apache/calcite/test/SqlOperatorTest.java | 3 +-
12 files changed, 243 insertions(+), 42 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlSyntax.java
b/core/src/main/java/org/apache/calcite/sql/SqlSyntax.java
index b1825d5dc2..826891794d 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlSyntax.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlSyntax.java
@@ -147,6 +147,23 @@ public enum SqlSyntax {
}
},
+ /**
+ * Function syntax which takes no parentheses and return specific constant
value, for
+ * example "PI".
+ *
+ * @see SqlConformance#allowNiladicConstantWithoutParentheses()
+ */
+ FUNCTION_ID_CONSTANT(FUNCTION) {
+ @Override public void unparse(
+ SqlWriter writer,
+ SqlOperator operator,
+ SqlCall call,
+ int leftPrec,
+ int rightPrec) {
+ SqlUtil.unparseFunctionSyntax(operator, writer, call, false);
+ }
+ },
+
/**
* Syntax of an internal operator, which does not appear in the SQL.
*/
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
index b507a50244..97b095d2f6 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java
@@ -336,6 +336,7 @@ public abstract class SqlUtil {
// when it has 0 args, not "LOCALTIME()".
return;
case FUNCTION_STAR: // E.g. "COUNT(*)"
+ case FUNCTION_ID_CONSTANT: // E.g. "PI()"
case FUNCTION: // E.g. "RANK()"
case ORDERED_FUNCTION: // E.g. "STRING_AGG(x)"
// fall through - dealt with below
@@ -406,7 +407,8 @@ public abstract class SqlUtil {
// with empty argument list, e.g. LOCALTIME, we should not quote
// such identifier cause quoted `LOCALTIME` always represents a sql
identifier.
if (asFunctionID
- || operator.getSyntax() == SqlSyntax.FUNCTION_ID) {
+ || operator.getSyntax() == SqlSyntax.FUNCTION_ID
+ || operator.getSyntax() == SqlSyntax.FUNCTION_ID_CONSTANT) {
writer.keyword(identifier.getSimple());
unparsedAsFunc = true;
}
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 75ce64286b..5856288057 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
@@ -28,6 +28,7 @@ import org.apache.calcite.sql.SqlDateLiteral;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.SqlTimeLiteral;
import org.apache.calcite.sql.SqlTimestampLiteral;
import org.apache.calcite.sql.SqlUtil;
@@ -170,44 +171,48 @@ public class OracleSqlDialect extends SqlDialect {
if (call.getOperator() == SqlStdOperatorTable.SUBSTRING) {
SqlUtil.unparseFunctionSyntax(SqlLibraryOperators.SUBSTR_ORACLE, writer,
call, false);
- } else {
- switch (call.getKind()) {
- case POSITION:
- final SqlWriter.Frame frame = writer.startFunCall("INSTR");
- writer.sep(",");
- call.operand(1).unparse(writer, leftPrec, rightPrec);
- writer.sep(",");
- call.operand(0).unparse(writer, leftPrec, rightPrec);
- if (3 == call.operandCount()) {
- writer.sep(",");
- call.operand(2).unparse(writer, leftPrec, rightPrec);
- }
- if (4 == call.operandCount()) {
- writer.sep(",");
- call.operand(2).unparse(writer, leftPrec, rightPrec);
- writer.sep(",");
- call.operand(3).unparse(writer, leftPrec, rightPrec);
- }
- writer.endFunCall(frame);
- break;
- case FLOOR:
- if (call.operandCount() != 2) {
- super.unparseCall(writer, call, leftPrec, rightPrec);
- return;
- }
-
- final SqlLiteral timeUnitNode = call.operand(1);
- final TimeUnitRange timeUnit =
timeUnitNode.getValueAs(TimeUnitRange.class);
+ return;
+ }
- SqlCall call2 =
- SqlFloorFunction.replaceTimeUnitOperand(call, timeUnit.name(),
- timeUnitNode.getParserPosition());
- SqlFloorFunction.unparseDatetimeFunction(writer, call2, "TRUNC", true);
- break;
+ if (call.getOperator().getSyntax() == SqlSyntax.FUNCTION_ID_CONSTANT) {
+ writer.sep(call.getOperator().getName());
+ return;
+ }
- default:
+ switch (call.getKind()) {
+ case POSITION:
+ final SqlWriter.Frame frame = writer.startFunCall("INSTR");
+ writer.sep(",");
+ call.operand(1).unparse(writer, leftPrec, rightPrec);
+ writer.sep(",");
+ call.operand(0).unparse(writer, leftPrec, rightPrec);
+ if (3 == call.operandCount()) {
+ writer.sep(",");
+ call.operand(2).unparse(writer, leftPrec, rightPrec);
+ }
+ if (4 == call.operandCount()) {
+ writer.sep(",");
+ call.operand(2).unparse(writer, leftPrec, rightPrec);
+ writer.sep(",");
+ call.operand(3).unparse(writer, leftPrec, rightPrec);
+ }
+ writer.endFunCall(frame);
+ break;
+ case FLOOR:
+ if (call.operandCount() != 2) {
super.unparseCall(writer, call, leftPrec, rightPrec);
+ return;
}
+ final SqlLiteral timeUnitNode = call.operand(1);
+ final TimeUnitRange timeUnit =
timeUnitNode.getValueAs(TimeUnitRange.class);
+
+ SqlCall call2 =
+ SqlFloorFunction.replaceTimeUnitOperand(call, timeUnit.name(),
+ timeUnitNode.getParserPosition());
+ SqlFloorFunction.unparseDatetimeFunction(writer, call2, "TRUNC", true);
+ 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 9e0cfa92a2..a2e3d66504 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
@@ -1933,7 +1933,7 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
public static final SqlFunction PI =
SqlBasicFunction.create("PI", ReturnTypes.DOUBLE, OperandTypes.NILADIC,
SqlFunctionCategory.NUMERIC)
- .withSyntax(SqlSyntax.FUNCTION_ID);
+ .withSyntax(SqlSyntax.FUNCTION_ID_CONSTANT);
/** {@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/sql/validate/SqlAbstractConformance.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java
index 9ff957a805..bf73e6dd83 100644
---
a/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java
+++
b/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java
@@ -93,6 +93,10 @@ public abstract class SqlAbstractConformance implements
SqlConformance {
return SqlConformanceEnum.DEFAULT.allowNiladicParentheses();
}
+ @Override public boolean allowNiladicConstantWithoutParentheses() {
+ return SqlConformanceEnum.DEFAULT.allowNiladicConstantWithoutParentheses();
+ }
+
@Override public boolean allowExplicitRowValueConstructor() {
return SqlConformanceEnum.DEFAULT.allowExplicitRowValueConstructor();
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
index c38413e3dd..6909edcac2 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java
@@ -361,6 +361,30 @@ public interface SqlConformance {
*/
boolean allowNiladicParentheses();
+ /**
+ * Whether to allow parentheses to be specified in calls to niladic
functions of
+ * returned the specific constant value.
+ *
+ * <p>For example, {@code PI} is a niladic function and return specific
constant values pi.
+ * In standard SQL it must be invoked with parentheses:
+ *
+ * <blockquote><code>VALUES PI()</code></blockquote>
+ *
+ * <p>If {@code allowNiladicConstantWithoutParentheses}, the following
syntax is also valid:
+ *
+ * <blockquote><code>VALUES PI</code></blockquote>
+ *
+ * <p>The same function include E which result is Euler's constant.
+ *
+ * <p>Among the built-in conformance levels, true in
+ * {@link SqlConformanceEnum#ORACLE_10},
+ * {@link SqlConformanceEnum#ORACLE_12},
+ * {@link SqlConformanceEnum#DEFAULT};
+ * {@link SqlConformanceEnum#LENIENT};
+ * false otherwise.
+ */
+ boolean allowNiladicConstantWithoutParentheses();
+
/**
* Whether to allow SQL syntax "{@code ROW(expr1, expr2, expr3)}".
*
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
index 352b1930d3..d5af742e5d 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java
@@ -300,6 +300,19 @@ public enum SqlConformanceEnum implements SqlConformance {
}
}
+ @Override public boolean allowNiladicConstantWithoutParentheses() {
+ switch (this) {
+ case ORACLE_10:
+ case ORACLE_12:
+ case DEFAULT:
+ case LENIENT:
+ case BABEL:
+ return true;
+ default:
+ return false;
+ }
+ }
+
@Override public boolean allowExplicitRowValueConstructor() {
switch (this) {
case DEFAULT:
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java
index 1c9d033843..aecf713750 100644
---
a/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java
+++
b/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java
@@ -107,6 +107,9 @@ public class SqlDelegatingConformance implements
SqlConformance {
return delegate.allowNiladicParentheses();
}
+ @Override public boolean allowNiladicConstantWithoutParentheses() {
+ return delegate.allowNiladicConstantWithoutParentheses();
+ }
@Override public boolean allowExplicitRowValueConstructor() {
return delegate.allowExplicitRowValueConstructor();
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 0a80a414da..f216081fce 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -1964,13 +1964,21 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
opTab.lookupOperatorOverloads(id, null, SqlSyntax.FUNCTION, list,
catalogReader.nameMatcher());
for (SqlOperator operator : list) {
- if (operator.getSyntax() == SqlSyntax.FUNCTION_ID) {
+ if (operator.getSyntax() == SqlSyntax.FUNCTION_ID
+ || operator.getSyntax() == SqlSyntax.FUNCTION_ID_CONSTANT) {
// Even though this looks like an identifier, it is a
// actually a call to a function. Construct a fake
// call to this function, so we can use the regular
// operator validation.
- return new SqlBasicCall(operator, ImmutableList.of(),
- id.getParserPosition(), null).withExpanded(true);
+ SqlCall sqlCall =
+ new SqlBasicCall(operator, ImmutableList.of(),
id.getParserPosition(), null)
+ .withExpanded(true);
+ if (operator.getSyntax() == SqlSyntax.FUNCTION_ID_CONSTANT
+ &&
!this.config.conformance().allowNiladicConstantWithoutParentheses()) {
+ throw handleUnresolvedFunction(sqlCall, operator,
+ ImmutableList.of(), null);
+ }
+ return sqlCall;
}
}
}
@@ -2072,7 +2080,8 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
if (overloads.size() == 1) {
SqlFunction fun = (SqlFunction) overloads.get(0);
if ((fun.getSqlIdentifier() == null)
- && (fun.getSyntax() != SqlSyntax.FUNCTION_ID)) {
+ && (fun.getSyntax() != SqlSyntax.FUNCTION_ID
+ && fun.getSyntax() != SqlSyntax.FUNCTION_ID_CONSTANT)) {
final int expectedArgCount =
fun.getOperandCountRange().getMin();
throw newValidationError(call,
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 2349380ed2..7e82ccf841 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,79 @@ class RelToSqlConverterTest {
.withStarRocks().ok(expectedStarRocks);
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-6566";>[CALCITE-6566]</a>
+ * JDBC adapter should generate PI function with parentheses in most
dialects. */
+ @Test void testPiFunction() {
+ String query = "select PI()";
+ final String expected = "SELECT PI()\n"
+ + "FROM (VALUES (0)) AS \"t\" (\"ZERO\")";
+ final String expectedHive = "SELECT PI()";
+ final String expectedSpark = "SELECT PI()\n"
+ + "FROM (VALUES (0)) `t` (`ZERO`)";
+ final String expectedMssql = "SELECT PI()\n"
+ + "FROM (VALUES (0)) AS [t] ([ZERO])";
+ 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\n"
+ + "FROM \"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 testPiFunctionWithoutParentheses() {
+ String query = "select PI";
+ 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`\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\"\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)
+ .withMysql().ok(expectedMysql)
+ .withClickHouse().ok(expectedClickHouse)
+ .withPresto().ok(expectedPresto)
+ .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"
diff --git a/core/src/test/resources/sql/misc.iq
b/core/src/test/resources/sql/misc.iq
index 08ecc00a6b..4fa8080b35 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -1837,6 +1837,16 @@ values pi;
!ok
+values pi();
++-------------------+
+| EXPR$0 |
++-------------------+
+| 3.141592653589793 |
++-------------------+
+(1 row)
+
+!ok
+
# DEGREES function
values (degrees(pi), degrees(-pi / 2));
+--------+--------+
@@ -2304,4 +2314,46 @@ GROUP BY floor(empno/2);
!ok
+# [CALCITE-6566] JDBC adapter should generate PI function with parentheses in
most dialects
+
+!use scott-mysql
+
+# PI function
+values pi;
+No match found for function signature PI()
+!error
+
+values pi();
++-------------------+
+| EXPR$0 |
++-------------------+
+| 3.141592653589793 |
++-------------------+
+(1 row)
+
+!ok
+
+!use scott
+
+# PI function
+values pi;
++-------------------+
+| PI |
++-------------------+
+| 3.141592653589793 |
++-------------------+
+(1 row)
+
+!ok
+
+values pi();
++-------------------+
+| EXPR$0 |
++-------------------+
+| 3.141592653589793 |
++-------------------+
+(1 row)
+
+!ok
+
# End misc.iq
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index cea089a2ee..c00af0ff84 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -9583,8 +9583,7 @@ public class SqlOperatorTest {
final SqlOperatorFixture f = fixture();
f.setFor(SqlStdOperatorTable.PI, VmName.EXPAND);
f.checkScalarApprox("PI", "DOUBLE NOT NULL", isWithin(3.1415d, 0.0001d));
- f.checkFails("^PI()^",
- "No match found for function signature PI\\(\\)", false);
+ f.checkScalarApprox("PI()", "DOUBLE NOT NULL", isWithin(3.1415d, 0.0001d));
// assert that PI function is not dynamic [CALCITE-2750]
assertThat("PI operator should not be identified as dynamic function",
