julianhyde commented on code in PR #3210:
URL: https://github.com/apache/calcite/pull/3210#discussion_r1198012023
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -593,6 +593,48 @@ private boolean hasFractionalPart(BigDecimal bd) {
}
};
+ public static final SqlSingleOperandTypeChecker
NUMERIC_UNIT_INTERVAL_NUMERIC_LITERAL =
Review Comment:
add javadoc. also add a blank line after the declaration.
##########
core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties:
##########
@@ -131,6 +131,7 @@ AggregateInWithinGroupIllegal=WITHIN GROUP must not contain
aggregate expression
AggregateInWithinDistinctIllegal=WITHIN DISTINCT must not contain aggregate
expression
AggregateMissingWithinGroupClause=Aggregate expression ''{0}'' must contain a
WITHIN GROUP clause
WithinGroupClauseIllegalInAggregate=Aggregate expression ''{0}'' must not
contain a WITHIN GROUP clause
+percentileFunctionsArgumentLimit=Percentile functions may only have 1 or 2
arguments
AggregateIllegalInOrderBy=Aggregate expression is illegal in ORDER BY clause
of non-aggregating SELECT
Review Comment:
s/p/P/
##########
site/_docs/reference.md:
##########
@@ -2836,6 +2836,8 @@ Dialect-specific aggregate functions.
| b | LOGICAL_OR(condition) | Synonym for `SOME`
| s | MAX_BY(value, comp) | Synonym for `ARG_MAX`
| s | MIN_BY(value, comp) | Synonym for `ARG_MIN`
+| b | PERCENTILE_CONT(value, fraction) | 2-argument synonym for
the standard `PERCENTILE_CONT` that does not use WITHIN GROUP clause and allows
for null treatment
+| b | PERCENTILE_DISC(value, fraction) | 2-argument synonym for
the standard `PERCENTILE_DISC` that does not use WITHIN GROUP clause and allows
for null treatment
Review Comment:
does it make sense to add `[ RESPECT NULLS ]` to the syntax?
is it possible to concisely show the mapping, e.g. that
`PERCENTILE_CONT(value, fraction)` is a synonym for `PERCENTILE_CONT(fraction)
WITHIN GROUP (ORDER BY value)` (or whatever)
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4056,6 +4056,59 @@ SqlCall StringAggFunctionCall() :
}
}
+/**
Review Comment:
please include some brief example syntax.
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -748,6 +748,50 @@ protected static SortedSet<String> keywords(@Nullable
String dialect) {
.fails("(?s)Encountered \"\\*\" at .*");
}
+ @Test void testPercentileCont() {
+ sql("select percentile_cont(.5) within group (order by 3) from t")
+ .ok("SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY 3)\n"
+ +
Review Comment:
'+' should be indented
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -6139,27 +6139,35 @@ private SqlNode navigationInDefine(SqlNode node, String
alpha) {
}
if (op.isPercentile()) {
Review Comment:
This code is now sufficiently complex that a couple of examples would be
useful.
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6540,6 +6540,64 @@ void testGroupExpressionEquivalenceParams() {
.fails("'PERCENTILE_DISC' requires precisely one ORDER BY key");
}
+ @Test void testPercentileFunctionsBigQuery() {
+ final SqlOperatorTable opTable = operatorTableFor(SqlLibrary.BIG_QUERY);
Review Comment:
the examples over `unnest(array)` are appropriate for `big-query.iq` but are
not typical of actual usage. for these tests can you switch to queries that use
`emp`. Compute the percentile of sal of each employee within their department
and within the whole org.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
