From 2db361401d1e1661eea6aeb51dc2275b0592fe10 Mon Sep 17 00:00:00 2001 From: Tanner Clary Date: Mon, 8 Jan 2024 22:30:20 -0800 Subject: [PATCH] [CALCITE-6196] Accept BigQuery PERCENTILE functions without OVER clause Address Mihai comment --- .../calcite/rel/core/AggregateCall.java | 5 ++-- .../calcite/sql/fun/SqlLibraryOperators.java | 2 -- .../sql/validate/SqlValidatorImpl.java | 3 +-- .../rel/rel2sql/RelToSqlConverterTest.java | 24 +++++++++++++++++++ 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java b/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java index fa0d978962f..bfe6b95e66c 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java +++ b/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java @@ -467,9 +467,8 @@ public Aggregate.AggCallBinding createBinding( final RelDataTypeFactory typeFactory = aggregateRelBase.getCluster().getTypeFactory(); - if (aggFunction.getKind() == SqlKind.PERCENTILE_DISC - || aggFunction.getKind() == SqlKind.PERCENTILE_CONT) { - assert collation.getKeys().size() == 1; + if (this.argList.size() == 1 && (aggFunction.getKind() == SqlKind.PERCENTILE_DISC + || aggFunction.getKind() == SqlKind.PERCENTILE_CONT)) { return new Aggregate.PercentileDiscAggCallBinding(typeFactory, aggFunction, SqlTypeUtil.projectTypes(rowType, argList), SqlTypeUtil.projectTypes(rowType, collation.getKeys()).get(0), 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 c679be0f898..088f3a3551e 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 @@ -711,7 +711,6 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding, ReturnTypes.DOUBLE, OperandTypes.NUMERIC_UNIT_INTERVAL_NUMERIC_LITERAL) .withFunctionType(SqlFunctionCategory.SYSTEM) - .withOver(true) .withPercentile(true) .withAllowsNullTreatment(true) .withAllowsFraming(false); @@ -726,7 +725,6 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding, ReturnTypes.ARG0, OperandTypes.NUMERIC_UNIT_INTERVAL_NUMERIC_LITERAL) .withFunctionType(SqlFunctionCategory.SYSTEM) - .withOver(true) .withPercentile(true) .withAllowsNullTreatment(true) .withAllowsFraming(false); 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 81f2140b6a4..bda32e67163 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 @@ -6209,7 +6209,7 @@ private SqlNode navigationInDefine(SqlNode node, String alpha) { // Because there are two forms of the PERCENTILE_CONT/PERCENTILE_DISC functions, // they are distinguished by their operand count and then validated accordingly. // For example, the standard single operand form requires group order while the - // 2-operand form allows for null treatment and requires an OVER() clause. + // 2-operand form allows for null treatment. if (op.isPercentile()) { switch (aggCall.operandCount()) { case 1: @@ -6235,7 +6235,6 @@ private SqlNode navigationInDefine(SqlNode node, String alpha) { break; case 2: assert op.allowsNullTreatment(); - assert op.requiresOver(); assert op.requiresGroupOrder() == Optionality.FORBIDDEN; break; default: 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 cb3910e38b3..7e575e34fcd 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 @@ -1295,6 +1295,30 @@ private static String toSql(RelNode root, SqlDialect dialect, .withMysql().ok(expectedMysql); } + /** Test case for + * [CALCITE-6196] + * Remove OVER requirement for BigQuery PERCENTILE_CONT/DISC. */ + @Test void testPercentileContFunctionWithoutOver() { + final String query = "select percentile_cont(\"product_id\", .5) " + + "from \"foodmart\".\"product\""; + final String expected = "SELECT PERCENTILE_CONT(product_id, 0.5)\n" + + "FROM foodmart.product"; + + sql(query).withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).ok(expected); + } + + /** Test case for + * [CALCITE-6196] + * Remove OVER requirement for BigQuery PERCENTILE_CONT/DISC. */ + @Test void testPercentileDiscFunctionWithoutOver() { + final String query = "select percentile_disc(\"product_id\", .5) " + + "from \"foodmart\".\"product\""; + final String expected = "SELECT PERCENTILE_DISC(product_id, 0.5)\n" + + "FROM foodmart.product"; + + sql(query).withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).ok(expected); + } + /** As {@link #testSum0BecomesCoalesce()} but for windowed aggregates. */ @Test void testWindowedSum0BecomesCoalesce() { final String query = "select\n"