Skip to content

Commit 9e73ebd

Browse files
committed
Add more ITs after merging push down limit to agg buckets
Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 9be3579 commit 9e73ebd

8 files changed

Lines changed: 110 additions & 46 deletions

File tree

integ-test/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ integTest {
451451

452452
dependsOn ':opensearch-sql-plugin:bundlePlugin'
453453
if(getOSFamilyType() != "windows") {
454-
dependsOn startPrometheus
454+
// dependsOn startPrometheus
455455
finalizedBy stopPrometheus
456456
}
457457

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,20 @@ public void testPushdownLimitIntoAggregation() throws IOException {
547547
"source=opensearch-sql_test_index_account | stats count() by state | sort state | head"
548548
+ " 100 | head 10 from 10 "));
549549

550+
expected = loadExpectedPlan("explain_limit_agg_pushdown_bucket_nullable1.json");
551+
assertJsonEqualsIgnoreId(
552+
expected,
553+
explainQueryToString(
554+
"source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by"
555+
+ " state | head 100 | head 10 from 10 "));
556+
557+
expected = loadExpectedPlan("explain_limit_agg_pushdown_bucket_nullable2.json");
558+
assertJsonEqualsIgnoreId(
559+
expected,
560+
explainQueryToString(
561+
"source=opensearch-sql_test_index_account | stats bucket_nullable=false count() by"
562+
+ " state | sort state | head 100 | head 10 from 10 "));
563+
550564
// Don't pushdown the combination of limit and sort
551565
expected = loadExpectedPlan("explain_limit_agg_pushdown5.json");
552566
assertJsonEqualsIgnoreId(
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"calcite": {
33
"logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalUnion(all=[true])\n LogicalProject(cnt=[$1], gender=[$0])\n LogicalAggregate(group=[{0}], cnt=[COUNT($1)])\n LogicalProject(gender=[$4], balance=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n LogicalProject(cnt=[$0], gender=[null:VARCHAR])\n LogicalAggregate(group=[{}], cnt=[COUNT()])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n",
4-
"physical": "EnumerableLimit(fetch=[10000])\n EnumerableUnion(all=[true])\n EnumerableCalc(expr#0..1=[{inputs}], cnt=[$t1], gender=[$t0])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={0},cnt=COUNT($1)), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"gender\":{\"terms\":{\"field\":\"gender.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"cnt\":{\"value_count\":{\"field\":\"balance\"}}}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n EnumerableLimit(fetch=[10000])\n EnumerableCalc(expr#0=[{inputs}], expr#1=[null:VARCHAR], proj#0..1=[{exprs}])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={},cnt=COUNT())], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"cnt\":{\"value_count\":{\"field\":\"_index\"}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
4+
"physical": "EnumerableLimit(fetch=[10000])\n EnumerableUnion(all=[true])\n EnumerableCalc(expr#0..1=[{inputs}], cnt=[$t1], gender=[$t0])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={0},cnt=COUNT($1)), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"gender\":{\"terms\":{\"field\":\"gender.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"cnt\":{\"value_count\":{\"field\":\"balance\"}}}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n EnumerableCalc(expr#0=[{inputs}], expr#1=[null:VARCHAR], proj#0..1=[{exprs}])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={},cnt=COUNT()), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"cnt\":{\"value_count\":{\"field\":\"_index\"}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
55
}
66
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"calcite": {
33
"logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalAggregate(group=[{}], cnt=[COUNT()])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])\n",
4-
"physical": "EnumerableLimit(fetch=[10000])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={},cnt=COUNT())], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"cnt\":{\"value_count\":{\"field\":\"_index\"}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
4+
"physical": "CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={},cnt=COUNT()), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"cnt\":{\"value_count\":{\"field\":\"_index\"}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
55
}
6-
}
6+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"calcite": {
3+
"logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalSort(offset=[10], fetch=[10])\n LogicalSort(fetch=[100])\n LogicalProject(count()=[$1], state=[$0])\n LogicalFilter(condition=[IS NOT NULL($0)])\n LogicalAggregate(group=[{0}], count()=[COUNT()])\n LogicalProject(state=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])\n",
4+
"physical": "EnumerableLimit(fetch=[10000])\n EnumerableCalc(expr#0..1=[{inputs}], count()=[$t1], state=[$t0])\n EnumerableLimit(offset=[10], fetch=[10])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[FILTER->IS NOT NULL($7), AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={0},count()=COUNT()), LIMIT->100, LIMIT->[10 from 10]], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"query\":{\"exists\":{\"field\":\"state\",\"boost\":1.0}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"size\":20,\"min_doc_count\":1,\"shard_min_doc_count\":0,\"show_term_doc_count_error\":false,\"order\":{\"_key\":\"asc\"}},\"aggregations\":{\"count()\":{\"value_count\":{\"field\":\"_index\"}}}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
5+
}
6+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"calcite": {
3+
"logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalSort(offset=[10], fetch=[10])\n LogicalSort(sort0=[$1], dir0=[ASC-nulls-first], fetch=[100])\n LogicalProject(count()=[$1], state=[$0])\n LogicalFilter(condition=[IS NOT NULL($0)])\n LogicalAggregate(group=[{0}], count()=[COUNT()])\n LogicalProject(state=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])\n",
4+
"physical": "EnumerableLimit(fetch=[10000])\n EnumerableCalc(expr#0..1=[{inputs}], count()=[$t1], state=[$t0])\n EnumerableLimit(offset=[10], fetch=[10])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[FILTER->IS NOT NULL($7), AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={0},count()=COUNT()), SORT->[0 ASC FIRST], LIMIT->100, LIMIT->[10 from 10]], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"query\":{\"exists\":{\"field\":\"state\",\"boost\":1.0}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"size\":20,\"min_doc_count\":1,\"shard_min_doc_count\":0,\"show_term_doc_count_error\":false,\"order\":{\"_key\":\"asc\"}},\"aggregations\":{\"count()\":{\"value_count\":{\"field\":\"_index\"}}}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
5+
}
6+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"calcite": {
33
"logical": "LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalAggregate(group=[{}], p50=[percentile_approx($0, $1, $2)], p90=[percentile_approx($0, $3, $2)])\n LogicalProject(balance=[$3], $f2=[50], $f3=[FLAG(BIGINT)], $f4=[90])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])\n",
4-
"physical": "EnumerableLimit(fetch=[10000])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={},p50=percentile_approx($0, $1, $2),p90=percentile_approx($0, $3, $2))], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"p50\":{\"percentiles\":{\"field\":\"balance\",\"percents\":[50.0],\"keyed\":true,\"tdigest\":{\"compression\":100.0}}},\"p90\":{\"percentiles\":{\"field\":\"balance\",\"percents\":[90.0],\"keyed\":true,\"tdigest\":{\"compression\":100.0}}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
4+
"physical": "CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[AGGREGATION->rel#:LogicalAggregate.NONE.[](input=RelSubset#,group={},p50=percentile_approx($0, $1, $2),p90=percentile_approx($0, $3, $2)), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"size\":0,\"timeout\":\"1m\",\"aggregations\":{\"p50\":{\"percentiles\":{\"field\":\"balance\",\"percents\":[50.0],\"keyed\":true,\"tdigest\":{\"compression\":100.0}}},\"p90\":{\"percentiles\":{\"field\":\"balance\",\"percents\":[90.0],\"keyed\":true,\"tdigest\":{\"compression\":100.0}}}}}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n"
55
}
66
}

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java

Lines changed: 79 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@
4444
import org.opensearch.search.aggregations.AggregationBuilder;
4545
import org.opensearch.search.aggregations.AggregationBuilders;
4646
import org.opensearch.search.aggregations.AggregatorFactories.Builder;
47+
import org.opensearch.search.aggregations.BucketOrder;
4748
import org.opensearch.search.aggregations.bucket.composite.CompositeAggregationBuilder;
4849
import org.opensearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder;
4950
import org.opensearch.search.aggregations.bucket.missing.MissingOrder;
51+
import org.opensearch.search.aggregations.bucket.terms.MultiTermsAggregationBuilder;
52+
import org.opensearch.search.aggregations.bucket.terms.TermsAggregationBuilder;
5053
import org.opensearch.search.aggregations.support.ValuesSourceAggregationBuilder;
5154
import org.opensearch.search.sort.ScoreSortBuilder;
5255
import org.opensearch.search.sort.SortBuilder;
@@ -397,53 +400,88 @@ public void apply(OpenSearchRequestBuilder requestBuilder) {
397400
}
398401

399402
public void pushDownSortIntoAggBucket(List<RelFieldCollation> collations) {
400-
// It will always use a single CompositeAggregationBuilder for the aggregation with GroupBy
401-
// See {@link AggregateAnalyzer}
402-
CompositeAggregationBuilder compositeAggregationBuilder =
403-
(CompositeAggregationBuilder) aggregationBuilder.getLeft().getFirst();
404-
List<CompositeValuesSourceBuilder<?>> buckets =
405-
((CompositeAggregationBuilder) aggregationBuilder.getLeft().getFirst()).sources();
406-
List<CompositeValuesSourceBuilder<?>> newBuckets = new ArrayList<>(buckets.size());
403+
AggregationBuilder builder = aggregationBuilder.getLeft().getFirst();
407404
List<Integer> selected = new ArrayList<>(collations.size());
408-
// Have to put the collation required buckets first, then the rest of buckets.
409-
collations.forEach(
410-
collation -> {
411-
CompositeValuesSourceBuilder<?> bucket = buckets.get(collation.getFieldIndex());
412-
Direction direction = collation.getDirection();
413-
NullDirection nullDirection = collation.nullDirection;
414-
SortOrder order =
415-
Direction.DESCENDING.equals(direction) ? SortOrder.DESC : SortOrder.ASC;
416-
MissingOrder missingOrder =
417-
switch (nullDirection) {
418-
case FIRST -> MissingOrder.FIRST;
419-
case LAST -> MissingOrder.LAST;
420-
default -> MissingOrder.DEFAULT;
421-
};
422-
newBuckets.add(bucket.order(order).missingOrder(missingOrder));
423-
selected.add(collation.getFieldIndex());
424-
});
425-
IntStream.range(0, buckets.size())
426-
.filter(i -> !selected.contains(i))
427-
.forEach(i -> newBuckets.add(buckets.get(i)));
428-
Builder newAggBuilder = new Builder();
429-
compositeAggregationBuilder.getSubAggregations().forEach(newAggBuilder::addAggregator);
430-
aggregationBuilder =
431-
Pair.of(
432-
Collections.singletonList(
433-
AggregationBuilders.composite("composite_buckets", newBuckets)
434-
.subAggregations(newAggBuilder)
435-
.size(AGGREGATION_BUCKET_SIZE)),
436-
aggregationBuilder.getRight());
405+
if (builder instanceof CompositeAggregationBuilder compositeAggBuilder) {
406+
// It will always use a single CompositeAggregationBuilder for the aggregation with GroupBy
407+
// See {@link AggregateAnalyzer}
408+
List<CompositeValuesSourceBuilder<?>> buckets = compositeAggBuilder.sources();
409+
List<CompositeValuesSourceBuilder<?>> newBuckets = new ArrayList<>(buckets.size());
410+
// Have to put the collation required buckets first, then the rest of buckets.
411+
collations.forEach(
412+
collation -> {
413+
CompositeValuesSourceBuilder<?> bucket = buckets.get(collation.getFieldIndex());
414+
Direction direction = collation.getDirection();
415+
NullDirection nullDirection = collation.nullDirection;
416+
SortOrder order =
417+
Direction.DESCENDING.equals(direction) ? SortOrder.DESC : SortOrder.ASC;
418+
MissingOrder missingOrder =
419+
switch (nullDirection) {
420+
case FIRST -> MissingOrder.FIRST;
421+
case LAST -> MissingOrder.LAST;
422+
default -> MissingOrder.DEFAULT;
423+
};
424+
newBuckets.add(bucket.order(order).missingOrder(missingOrder));
425+
selected.add(collation.getFieldIndex());
426+
});
427+
IntStream.range(0, buckets.size())
428+
.filter(i -> !selected.contains(i))
429+
.forEach(i -> newBuckets.add(buckets.get(i)));
430+
Builder newAggBuilder = new Builder();
431+
compositeAggBuilder.getSubAggregations().forEach(newAggBuilder::addAggregator);
432+
aggregationBuilder =
433+
Pair.of(
434+
Collections.singletonList(
435+
AggregationBuilders.composite("composite_buckets", newBuckets)
436+
.subAggregations(newAggBuilder)
437+
.size(AGGREGATION_BUCKET_SIZE)),
438+
aggregationBuilder.getRight());
439+
}
440+
if (builder instanceof TermsAggregationBuilder termsAggBuilder) {
441+
termsAggBuilder.order(
442+
BucketOrder.key(!collations.getFirst().getDirection().isDescending()));
443+
}
444+
// TODO for MultiTermsAggregationBuilder
437445
}
438446

447+
/**
448+
* Check if the limit can be pushed down into aggregation bucket when the limit size is less
449+
* than bucket number.
450+
*/
439451
public boolean pushDownLimitIntoBucketSize(Integer size) {
440-
CompositeAggregationBuilder compositeAggregationBuilder =
441-
(CompositeAggregationBuilder) aggregationBuilder.getLeft().getFirst();
442-
if (size < compositeAggregationBuilder.size()) {
443-
compositeAggregationBuilder.size(size);
452+
AggregationBuilder builder = aggregationBuilder.getLeft().getFirst();
453+
if (builder instanceof CompositeAggregationBuilder compositeAggBuilder) {
454+
if (size < compositeAggBuilder.size()) {
455+
compositeAggBuilder.size(size);
456+
return true;
457+
} else {
458+
return false;
459+
}
460+
}
461+
if (builder instanceof TermsAggregationBuilder termsAggBuilder) {
462+
if (size < termsAggBuilder.size()) {
463+
termsAggBuilder.size(size);
464+
return true;
465+
} else {
466+
return false;
467+
}
468+
}
469+
if (builder instanceof MultiTermsAggregationBuilder multiTermsAggBuilder) {
470+
if (size < multiTermsAggBuilder.size()) {
471+
multiTermsAggBuilder.size(size);
472+
return true;
473+
} else {
474+
return false;
475+
}
476+
}
477+
// now we only have Composite, Terms and MultiTerms bucket aggregations,
478+
// add code here when we could support more in the future.
479+
if (builder instanceof ValuesSourceAggregationBuilder.LeafOnly<?, ?>) {
480+
// Note: all metric aggregations will be treated as pushed since it generates only one row.
444481
return true;
445482
}
446-
return false;
483+
throw new OpenSearchRequestBuilder.PushDownUnSupportedException(
484+
"Unknown aggregation builder " + builder.getClass().getSimpleName());
447485
}
448486
}
449487
}

0 commit comments

Comments
 (0)