This is an automated email from the ASF dual-hosted git repository.
tanner 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 50a20824c4 [CALCITE-5955] BigQuery PERCENTILE functions are unparsed
incorrectly
50a20824c4 is described below
commit 50a20824c4536450dcae963b5e757cf4bbc7e406
Author: Tanner Clary <[email protected]>
AuthorDate: Thu Aug 24 13:31:54 2023 -0700
[CALCITE-5955] BigQuery PERCENTILE functions are unparsed incorrectly
---
.../apache/calcite/runtime/CalciteResource.java | 2 +-
.../calcite/sql/dialect/BigQuerySqlDialect.java | 5 +++
.../calcite/sql/fun/SqlBasicAggFunction.java | 39 +++++++++++++++-------
.../calcite/sql/fun/SqlLibraryOperators.java | 6 ++--
.../calcite/sql/fun/SqlStdOperatorTable.java | 6 ++--
.../calcite/runtime/CalciteResource.properties | 2 +-
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 36 ++++++++++++++++++++
.../org/apache/calcite/test/SqlValidatorTest.java | 2 +-
8 files changed, 79 insertions(+), 19 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index b32f752ad2..2791918382 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -490,7 +490,7 @@ public interface CalciteResource {
@BaseMessage("QUALIFY expression ''{0}'' must contain a window function")
ExInst<SqlValidatorException>
qualifyExpressionMustContainWindowFunction(String a0);
- @BaseMessage("ROW/RANGE not allowed with RANK, DENSE_RANK or ROW_NUMBER
functions")
+ @BaseMessage("ROW/RANGE not allowed with RANK, DENSE_RANK, ROW_NUMBER or
PERCENTILE_CONT/DISC functions")
ExInst<SqlValidatorException> rankWithFrame();
@BaseMessage("RANK or DENSE_RANK functions require ORDER BY clause in window
specification")
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
index c36e25a1da..5fa7382112 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java
@@ -209,6 +209,11 @@ public class BigQuerySqlDialect extends SqlDialect {
}
unparseItem(writer, call, leftPrec);
break;
+ case OVER:
+ call.operand(0).unparse(writer, leftPrec, rightPrec);
+ writer.keyword("OVER");
+ call.operand(1).unparse(writer, leftPrec, rightPrec);
+ break;
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicAggFunction.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicAggFunction.java
index 0c1009d098..b63b37cf29 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicAggFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicAggFunction.java
@@ -54,6 +54,7 @@ public final class SqlBasicAggFunction extends SqlAggFunction
{
private final boolean allowsNullTreatment;
private final boolean allowsSeparator;
private final boolean percentile;
+ private final boolean allowsFraming;
//~ Constructors -----------------------------------------------------------
@@ -66,7 +67,7 @@ public final class SqlBasicAggFunction extends SqlAggFunction
{
boolean requiresOrder, boolean requiresOver,
Optionality requiresGroupOrder, Optionality distinctOptionality,
SqlSyntax syntax, boolean allowsNullTreatment, boolean allowsSeparator,
- boolean percentile) {
+ boolean percentile, boolean allowsFraming) {
super(name, sqlIdentifier, kind,
requireNonNull(returnTypeInference, "returnTypeInference"),
operandTypeInference,
requireNonNull(operandTypeChecker, "operandTypeChecker"),
@@ -79,6 +80,7 @@ public final class SqlBasicAggFunction extends SqlAggFunction
{
this.allowsNullTreatment = allowsNullTreatment;
this.allowsSeparator = allowsSeparator;
this.percentile = percentile;
+ this.allowsFraming = allowsFraming;
}
/** Creates a SqlBasicAggFunction whose name is the same as its kind. */
@@ -95,7 +97,7 @@ public final class SqlBasicAggFunction extends SqlAggFunction
{
return new SqlBasicAggFunction(name, null, kind, returnTypeInference, null,
operandTypeChecker, null, SqlFunctionCategory.NUMERIC, false, false,
Optionality.FORBIDDEN, Optionality.OPTIONAL, SqlSyntax.FUNCTION, false,
- false, false);
+ false, false, true);
}
//~ Methods ----------------------------------------------------------------
@@ -147,7 +149,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
/** Sets {@link #getDistinctOptionality()}. */
@@ -156,7 +158,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
/** Sets {@link #getFunctionType()}. */
@@ -165,7 +167,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, category, requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
@Override public SqlSyntax getSyntax() {
@@ -178,7 +180,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
@Override public boolean allowsNullTreatment() {
@@ -191,7 +193,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
/** Returns whether this aggregate function allows '{@code SEPARATOR string}'
@@ -206,7 +208,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
@Override public boolean isPercentile() {
@@ -219,7 +221,20 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
+ }
+
+ @Override public boolean allowsFraming() {
+ return allowsFraming;
+ }
+
+ /** Sets {@link #allowsFraming()}. */
+ public SqlBasicAggFunction withAllowsFraming(boolean allowsFraming) {
+ return new SqlBasicAggFunction(getName(), getSqlIdentifier(), kind,
+ getReturnTypeInference(), getOperandTypeInference(),
+ getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
+ requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
/** Sets {@link #requiresOver()}. */
@@ -228,7 +243,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
over, requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
/** Sets {@link #requiresGroupOrder()}. */
@@ -237,7 +252,7 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), groupOrder, distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
/** Sets that value to be returned when {@link #unwrap} is applied to
@@ -247,6 +262,6 @@ public final class SqlBasicAggFunction extends
SqlAggFunction {
getReturnTypeInference(), getOperandTypeInference(),
getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
- allowsNullTreatment, allowsSeparator, percentile);
+ allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
}
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
index 427e305d50..0abce560e5 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
@@ -699,7 +699,8 @@ public abstract class SqlLibraryOperators {
.withFunctionType(SqlFunctionCategory.SYSTEM)
.withOver(true)
.withPercentile(true)
- .withAllowsNullTreatment(true);
+ .withAllowsNullTreatment(true)
+ .withAllowsFraming(false);
/** The {@code PERCENTILE_DISC} function, BigQuery's
* equivalent to {@link SqlStdOperatorTable#PERCENTILE_DISC},
@@ -713,7 +714,8 @@ public abstract class SqlLibraryOperators {
.withFunctionType(SqlFunctionCategory.SYSTEM)
.withOver(true)
.withPercentile(true)
- .withAllowsNullTreatment(true);
+ .withAllowsNullTreatment(true)
+ .withAllowsFraming(false);
/** The "DATE" function. It has the following overloads:
*
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 e6ee9734e0..40ec61d624 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
@@ -2265,7 +2265,8 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
OperandTypes.UNIT_INTERVAL_NUMERIC_LITERAL)
.withFunctionType(SqlFunctionCategory.SYSTEM)
.withGroupOrder(Optionality.MANDATORY)
- .withPercentile(true);
+ .withPercentile(true)
+ .withAllowsFraming(false);
/**
* {@code PERCENTILE_DISC} inverse distribution aggregate function.
@@ -2280,7 +2281,8 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
OperandTypes.UNIT_INTERVAL_NUMERIC_LITERAL)
.withFunctionType(SqlFunctionCategory.SYSTEM)
.withGroupOrder(Optionality.MANDATORY)
- .withPercentile(true);
+ .withPercentile(true)
+ .withAllowsFraming(false);
/**
* The LISTAGG operator. String aggregator function.
diff --git
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index ba13eabd2c..10aae01fb0 100644
---
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -168,7 +168,7 @@ DuplicateWindowName=Duplicate window names not allowed
EmptyWindowSpec=Empty window specification not allowed
DupWindowSpec=Duplicate window specification not allowed in the same window
clause
QualifyExpressionMustContainWindowFunction=QUALIFY expression ''{0}'' must
contain a window function
-RankWithFrame=ROW/RANGE not allowed with RANK, DENSE_RANK or ROW_NUMBER
functions
+RankWithFrame=ROW/RANGE not allowed with RANK, DENSE_RANK, ROW_NUMBER or
PERCENTILE_CONT/DISC functions
FuncNeedsOrderBy=RANK or DENSE_RANK functions require ORDER BY clause in
window specification
PartitionNotAllowed=PARTITION BY not allowed with existing window reference
OrderByOverlap=ORDER BY not allowed in both base and referenced windows
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 9eeabcceae..ba235f3d6f 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
@@ -1312,6 +1312,42 @@ class RelToSqlConverterTest {
.withPostgresql().ok(expectedPostgresql);
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5955";>[CALCITE-5955]
+ * BigQuery PERCENTILE functions are unparsed incorrectly</a>. */
+ @Test void testPercentileContWindow() {
+ final String partitionQuery = "select percentile_cont(\"product_id\",
0.5)\n"
+ + "over(partition by \"product_id\")\n"
+ + "from \"foodmart\".\"product\"";
+ final String expectedPartition = "SELECT PERCENTILE_CONT(product_id, 0.5) "
+ + "OVER (PARTITION BY product_id)\n"
+ + "FROM foodmart.product";
+ final String query = "select percentile_cont(\"product_id\", 0.5) over()\n"
+ + "from \"foodmart\".\"product\"";
+ final String expectedQuery = "SELECT PERCENTILE_CONT(product_id, 0.5) OVER
()\n"
+ + "FROM foodmart.product";
+
sql(partitionQuery).withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).ok(expectedPartition);
+
sql(query).withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).ok(expectedQuery);
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5955";>[CALCITE-5955]
+ * BigQuery PERCENTILE functions are unparsed incorrectly</a>. */
+ @Test void testPercentileDiscWindowFrameClause() {
+ final String partitionQuery = "select percentile_disc(\"product_id\",
0.5)\n"
+ + "over(partition by \"product_id\")\n"
+ + "from \"foodmart\".\"product\"";
+ final String expectedPartition = "SELECT PERCENTILE_DISC(product_id, 0.5) "
+ + "OVER (PARTITION BY product_id)\n"
+ + "FROM foodmart.product";
+ final String query = "select percentile_disc(\"product_id\", 0.5) over()\n"
+ + "from \"foodmart\".\"product\"";
+ final String expectedQuery = "SELECT PERCENTILE_DISC(product_id, 0.5) OVER
()\n"
+ + "FROM foodmart.product";
+
sql(partitionQuery).withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).ok(expectedPartition);
+
sql(query).withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).ok(expectedQuery);
+ }
+
/** Test case for
* <a
href="https://issues.apache.org/jira/browse/CALCITE-2722";>[CALCITE-2722]
* SqlImplementor createLeftCall method throws StackOverflowError</a>. */
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 4dc78de70d..3e9269b783 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -150,7 +150,7 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
"Set operator cannot combine streaming and non-streaming inputs";
private static final String ROW_RANGE_NOT_ALLOWED_WITH_RANK =
- "ROW/RANGE not allowed with RANK, DENSE_RANK or ROW_NUMBER functions";
+ "ROW/RANGE not allowed with RANK, DENSE_RANK, ROW_NUMBER or
PERCENTILE_CONT/DISC functions";
private static final String RANK_REQUIRES_ORDER_BY = "RANK or DENSE_RANK "
+ "functions require ORDER BY clause in window specification";
