Skip to content

Commit

Permalink
SQL: Fix issue with complex expression as args of PERCENTILE/_RANK (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
matriv committed Jan 24, 2019
1 parent f6b6f92 commit 74b6f30
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//

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

gender:s | p1:d
null |10019.0
Expand Down Expand Up @@ -48,7 +48,7 @@ M |10084.349 |10093.502
;

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

gender:s | rank:d
null |100.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ protected Iterable<RuleExecutor<LogicalPlan>.Batch> batches() {
Batch label = new Batch("Set as Optimized", Limiter.ONCE,
new SetAsOptimized());

return Arrays.asList(aggregate, operators, local, label);
return Arrays.asList(operators, aggregate, local, label);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunctionAttribute;
import org.elasticsearch.xpack.sql.optimizer.Optimizer;
import org.elasticsearch.xpack.sql.parser.SqlParser;
import org.elasticsearch.xpack.sql.plan.physical.EsQueryExec;
Expand Down Expand Up @@ -316,4 +317,24 @@ public void testConcatIsNotFoldedForNull() {
assertEquals(1, ee.output().size());
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
}

public void testFoldingOfPercentileSecondArgument() {
PhysicalPlan p = plan("SELECT PERCENTILE(int, 1 + 2) FROM test");
assertEquals(EsQueryExec.class, p.getClass());
EsQueryExec ee = (EsQueryExec) p;
assertEquals(1, ee.output().size());
assertEquals(AggregateFunctionAttribute.class, ee.output().get(0).getClass());
AggregateFunctionAttribute afa = (AggregateFunctionAttribute) ee.output().get(0);
assertThat(afa.propertyPath(), endsWith("[3.0]"));
}

public void testFoldingOfPercentileRankSecondArgument() {
PhysicalPlan p = plan("SELECT PERCENTILE_RANK(int, 1 + 2) FROM test");
assertEquals(EsQueryExec.class, p.getClass());
EsQueryExec ee = (EsQueryExec) p;
assertEquals(1, ee.output().size());
assertEquals(AggregateFunctionAttribute.class, ee.output().get(0).getClass());
AggregateFunctionAttribute afa = (AggregateFunctionAttribute) ee.output().get(0);
assertThat(afa.propertyPath(), endsWith("[3.0]"));
}
}

0 comments on commit 74b6f30

Please sign in to comment.