Skip to content

Commit 4a1e66e

Browse files
committed
SQL: Fix issue with complex expression as args of PERCENTILE/_RANK (#37102)
When the arguements of PERCENTILE and PERCENTILE_RANK can be folded, the `ConstantFolding` rule kicks in and calls the `replaceChildren()` method on `InnerAggregate` which is created from the aggregation rules of the `Optimizerz. `InnerAggregate` in turn, cannot implement the method as the logic of creating a new `InnerAggregate` instance from a list of `Expression`s resides in the Optimizer. So, instead, `ConstantFolding` should be applied before any of the aggregations related rules. Fixes: #37099
1 parent da50d75 commit 4a1e66e

File tree

3 files changed

+24
-3
lines changed

3 files changed

+24
-3
lines changed

x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//
44

55
singlePercentileWithoutComma
6-
SELECT gender, PERCENTILE(emp_no, 97) p1 FROM test_emp GROUP BY gender;
6+
SELECT gender, PERCENTILE(emp_no, 90 + 7) p1 FROM test_emp GROUP BY gender;
77

88
gender:s | p1:d
99
null |10019.0
@@ -48,7 +48,7 @@ M |10084.349 |10093.502
4848
;
4949

5050
percentileRank
51-
SELECT gender, PERCENTILE_RANK(emp_no, 10025) rank FROM test_emp GROUP BY gender;
51+
SELECT gender, PERCENTILE_RANK(emp_no, 10000 + 25) rank FROM test_emp GROUP BY gender;
5252

5353
gender:s | rank:d
5454
null |100.0

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ protected Iterable<RuleExecutor<LogicalPlan>.Batch> batches() {
159159
Batch label = new Batch("Set as Optimized", Limiter.ONCE,
160160
new SetAsOptimized());
161161

162-
return Arrays.asList(aggregate, operators, local, label);
162+
return Arrays.asList(operators, aggregate, local, label);
163163
}
164164

165165

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
1313
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
1414
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
15+
import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunctionAttribute;
1516
import org.elasticsearch.xpack.sql.optimizer.Optimizer;
1617
import org.elasticsearch.xpack.sql.parser.SqlParser;
1718
import org.elasticsearch.xpack.sql.plan.physical.EsQueryExec;
@@ -316,4 +317,24 @@ public void testConcatIsNotFoldedForNull() {
316317
assertEquals(1, ee.output().size());
317318
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
318319
}
320+
321+
public void testFoldingOfPercentileSecondArgument() {
322+
PhysicalPlan p = plan("SELECT PERCENTILE(int, 1 + 2) FROM test");
323+
assertEquals(EsQueryExec.class, p.getClass());
324+
EsQueryExec ee = (EsQueryExec) p;
325+
assertEquals(1, ee.output().size());
326+
assertEquals(AggregateFunctionAttribute.class, ee.output().get(0).getClass());
327+
AggregateFunctionAttribute afa = (AggregateFunctionAttribute) ee.output().get(0);
328+
assertThat(afa.propertyPath(), endsWith("[3.0]"));
329+
}
330+
331+
public void testFoldingOfPercentileRankSecondArgument() {
332+
PhysicalPlan p = plan("SELECT PERCENTILE_RANK(int, 1 + 2) FROM test");
333+
assertEquals(EsQueryExec.class, p.getClass());
334+
EsQueryExec ee = (EsQueryExec) p;
335+
assertEquals(1, ee.output().size());
336+
assertEquals(AggregateFunctionAttribute.class, ee.output().get(0).getClass());
337+
AggregateFunctionAttribute afa = (AggregateFunctionAttribute) ee.output().get(0);
338+
assertThat(afa.propertyPath(), endsWith("[3.0]"));
339+
}
319340
}

0 commit comments

Comments
 (0)