You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by li...@apache.org on 2022/12/22 11:38:32 UTC

[calcite] branch main updated: [CALCITE-5408] Return type of PERCENTILE_CONT should be the same as sort expression

This is an automated email from the ASF dual-hosted git repository.

libenchao 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 273d33dd61 [CALCITE-5408] Return type of PERCENTILE_CONT should be the same as sort expression
273d33dd61 is described below

commit 273d33dd61e0f1ec6b58e3b6381d6531608ee5e7
Author: zoudan <zo...@bytedance.com>
AuthorDate: Tue Dec 20 21:17:20 2022 +0800

    [CALCITE-5408] Return type of PERCENTILE_CONT should be the same as sort expression
    
    Close #3010
---
 core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java  | 3 ++-
 .../main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java   | 3 ++-
 .../main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java  | 7 ++++---
 core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java    | 2 +-
 core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java   | 2 +-
 testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java | 2 +-
 6 files changed, 11 insertions(+), 8 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 230b79fcc1..b1cdcbc57f 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
@@ -429,7 +429,8 @@ public class AggregateCall {
       Aggregate aggregateRelBase) {
     final RelDataType rowType = aggregateRelBase.getInput().getRowType();
 
-    if (aggFunction.getKind() == SqlKind.PERCENTILE_DISC) {
+    if (aggFunction.getKind() == SqlKind.PERCENTILE_DISC
+        || aggFunction.getKind() == SqlKind.PERCENTILE_CONT) {
       assert collation.getKeys().size() == 1;
       return new Aggregate.PercentileDiscAggCallBinding(
           aggregateRelBase.getCluster().getTypeFactory(), aggFunction,
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java
index 9024a21bd7..87f6fb7b8f 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlWithinGroupOperator.java
@@ -91,7 +91,8 @@ public class SqlWithinGroupOperator extends SqlBinaryOperator {
       throw validator.newValidationError(call, RESOURCE.withinGroupNotAllowed(operator.getName()));
     }
 
-    if (inner.getOperator().getKind() == SqlKind.PERCENTILE_DISC) {
+    if (inner.getOperator().getKind() == SqlKind.PERCENTILE_DISC
+        || inner.getOperator().getKind() == SqlKind.PERCENTILE_CONT) {
       // We first check the percentile call operands, and then derive the correct type using
       // PercentileDiscCallBinding (See CALCITE-5230).
       SqlCallBinding opBinding =
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 e51e72b4bf..8a867471d7 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
@@ -2191,11 +2191,12 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable {
    * {@code PERCENTILE_CONT} inverse distribution aggregate function.
    *
    * <p>The argument must be a numeric literal in the range 0 to 1 inclusive
-   * (representing a percentage), and the return type is {@code DOUBLE}.
+   * (representing a percentage), and the return type is the type of the
+   * {@code ORDER BY} expression.
    */
   public static final SqlAggFunction PERCENTILE_CONT =
       SqlBasicAggFunction
-          .create(SqlKind.PERCENTILE_CONT, ReturnTypes.DOUBLE,
+          .create(SqlKind.PERCENTILE_CONT, ReturnTypes.PERCENTILE_DISC_CONT,
               OperandTypes.UNIT_INTERVAL_NUMERIC_LITERAL)
           .withFunctionType(SqlFunctionCategory.SYSTEM)
           .withGroupOrder(Optionality.MANDATORY)
@@ -2210,7 +2211,7 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable {
    */
   public static final SqlAggFunction PERCENTILE_DISC =
       SqlBasicAggFunction
-          .create(SqlKind.PERCENTILE_DISC, ReturnTypes.PERCENTILE_DISC,
+          .create(SqlKind.PERCENTILE_DISC, ReturnTypes.PERCENTILE_DISC_CONT,
               OperandTypes.UNIT_INTERVAL_NUMERIC_LITERAL)
           .withFunctionType(SqlFunctionCategory.SYSTEM)
           .withGroupOrder(Optionality.MANDATORY)
diff --git a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
index 17571c2e3d..d1ae67f1f4 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
@@ -1002,6 +1002,6 @@ public abstract class ReturnTypes {
     }
   };
 
-  public static final SqlReturnTypeInference PERCENTILE_DISC = opBinding ->
+  public static final SqlReturnTypeInference PERCENTILE_DISC_CONT = opBinding ->
       opBinding.getCollationType();
 }
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 de742d4fd1..bad43e5ddd 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -7935,7 +7935,7 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
         + "from emp\n"
         + "group by deptno";
     sql(sql)
-        .type("RecordType(DOUBLE NOT NULL C, INTEGER NOT NULL D) NOT NULL");
+        .type("RecordType(INTEGER NOT NULL C, INTEGER NOT NULL D) NOT NULL");
   }
 
   /** Tests that {@code PERCENTILE_CONT} only allows numeric fields. */
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 2151ad08f4..92cec17ff3 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -8033,7 +8033,7 @@ public class SqlOperatorTest {
     final SqlOperatorFixture f = fixture();
     f.setFor(SqlStdOperatorTable.PERCENTILE_CONT, VM_FENNEL, VM_JAVA);
     f.checkType("percentile_cont(0.25) within group (order by 1)",
-        "DOUBLE NOT NULL");
+        "INTEGER NOT NULL");
     f.checkFails("percentile_cont(0.25) within group (^order by 'a'^)",
         "Invalid type 'CHAR' in ORDER BY clause of 'PERCENTILE_CONT' function. "
             + "Only NUMERIC types are supported", false);