[CALCITE-5955] BigQuery PERCENTILE functions are unparsed incorrectly

core/src/main/java/org/apache/calcite/runtime/CalciteResource.java

@@ -487,7 +487,7 @@ ExInst<SqlValidatorException> intervalFieldExceedsPrecision(Number a0,
   @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, PERCENTILE_CONT/DISC or ROW_NUMBER functions")
   ExInst<SqlValidatorException> rankWithFrame();
 
   @BaseMessage("RANK or DENSE_RANK functions require ORDER BY clause in window specification")

core/src/main/java/org/apache/calcite/sql/dialect/BigQuerySqlDialect.java

@@ -209,6 +209,11 @@ public BigQuerySqlDialect(SqlDialect.Context context) {
       }
       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);
     }

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 @@ private SqlBasicAggFunction(String name, @Nullable SqlIdentifier sqlIdentifier,
       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 @@ private SqlBasicAggFunction(String name, @Nullable SqlIdentifier sqlIdentifier,
     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 static SqlBasicAggFunction create(String name, SqlKind kind,
     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 SqlAggFunction withName(String name) {
         getReturnTypeInference(), getOperandTypeInference(),
         getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
         requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
-        allowsNullTreatment, allowsSeparator, percentile);
+        allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
   }
 
   /** Sets {@link #getDistinctOptionality()}. */
@@ -156,7 +158,7 @@ SqlBasicAggFunction withDistinct(Optionality distinctOptionality) {
         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 SqlBasicAggFunction withFunctionType(SqlFunctionCategory category) {
         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 SqlBasicAggFunction withSyntax(SqlSyntax syntax) {
         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 SqlBasicAggFunction withAllowsNullTreatment(boolean allowsNullTreatment)
         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 SqlBasicAggFunction withAllowsSeparator(boolean allowsSeparator) {
         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 SqlBasicAggFunction withPercentile(boolean percentile) {
         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 SqlBasicAggFunction withOver(boolean over) {
         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 SqlBasicAggFunction withGroupOrder(Optionality groupOrder) {
         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 SqlBasicAggFunction withStatic(SqlStaticAggFunction staticFun) {
         getReturnTypeInference(), getOperandTypeInference(),
         getOperandTypeChecker(), staticFun, getFunctionType(), requiresOrder(),
         requiresOver(), requiresGroupOrder(), distinctOptionality, syntax,
-        allowsNullTreatment, allowsSeparator, percentile);
+        allowsNullTreatment, allowsSeparator, percentile, allowsFraming);
   }
 }

core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java

@@ -699,7 +699,8 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
           .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 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
           .withFunctionType(SqlFunctionCategory.SYSTEM)
           .withOver(true)
           .withPercentile(true)
-          .withAllowsNullTreatment(true);
+          .withAllowsNullTreatment(true)
+          .withAllowsFraming(false);
 
   /** The "DATE" function. It has the following overloads:
    *

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.

core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties

@@ -167,7 +167,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, PERCENTILE_CONT/DISC or ROW_NUMBER 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

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

@@ -1312,6 +1312,42 @@ private static String toSql(RelNode root, SqlDialect dialect,
         .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 expected = "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(expected);
+  }
+
+  /** 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 expected = "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(expected);
+  }
+
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-2722">[CALCITE-2722]
    * SqlImplementor createLeftCall method throws StackOverflowError</a>. */

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, PERCENTILE_CONT/DISC or ROW_NUMBER functions";
 
   private static final String RANK_REQUIRES_ORDER_BY = "RANK or DENSE_RANK "
       + "functions require ORDER BY clause in window specification";