Skip to content

Commit 3d0e782

Browse files
authored
Don't load global ordinals with the map execution_hint (#38158)
The terms aggregator loads the global ordinals to retrieve the cardinality of the field to aggregate on. This information is then used to select the strategy to use for the aggregation (breadth_first or depth_first). However this should be avoided if the execution_hint is explicitly set to map since this mode doesn't really need the global ordinals. Since we still need the cardinality of the field this change picks the maximum cardinality in the segments as an estimation of the total cardinality to select the strategy to use (breadth_first or depth_first). This estimation is only used if the execution hint is set to map, otherwise the global ordinals are still used to retrieve the accurate cardinality. Closes #37705
1 parent 1094d3b commit 3d0e782

File tree

2 files changed

+68
-1
lines changed

2 files changed

+68
-1
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/20_terms.yml

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,3 +748,70 @@ setup:
748748
- is_false: aggregations.str_terms.buckets.1.key_as_string
749749

750750
- match: { aggregations.str_terms.buckets.1.doc_count: 2 }
751+
752+
---
753+
"Global ordinals are not loaded with the map execution hint":
754+
755+
- skip:
756+
version: " - 6.6.99"
757+
reason: bug fixed in 6.7
758+
759+
- do:
760+
index:
761+
refresh: true
762+
index: test_1
763+
type: test
764+
id: 1
765+
routing: 1
766+
body: { "str": "abc" }
767+
768+
- do:
769+
index:
770+
refresh: true
771+
index: test_1
772+
type: test
773+
id: 2
774+
routing: 1
775+
body: { "str": "abc" }
776+
777+
- do:
778+
index:
779+
refresh: true
780+
index: test_1
781+
type: test
782+
id: 3
783+
routing: 1
784+
body: { "str": "bcd" }
785+
786+
- do:
787+
indices.refresh: {}
788+
789+
- do:
790+
search:
791+
index: test_1
792+
body: { "size" : 0, "aggs" : { "str_terms" : { "terms" : { "field" : "str", "execution_hint" : "map" } } } }
793+
794+
- length: { aggregations.str_terms.buckets: 2 }
795+
796+
- do:
797+
indices.stats:
798+
index: test_1
799+
metric: fielddata
800+
fielddata_fields: str
801+
802+
- match: { indices.test_1.total.fielddata.memory_size_in_bytes: 0}
803+
804+
- do:
805+
search:
806+
index: test_1
807+
body: { "size" : 0, "aggs" : { "str_terms" : { "terms" : { "field" : "str", "execution_hint" : "global_ordinals" } } } }
808+
809+
- length: { aggregations.str_terms.buckets: 2 }
810+
811+
- do:
812+
indices.stats:
813+
index: test_1
814+
metric: fielddata
815+
fielddata_fields: str
816+
817+
- gt: { indices.test_1.total.fielddata.memory_size_in_bytes: 0}

server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, Aggregator pare
133133
if (valuesSource instanceof ValuesSource.Bytes.WithOrdinals == false) {
134134
execution = ExecutionMode.MAP;
135135
}
136-
final long maxOrd = getMaxOrd(valuesSource, context.searcher());
136+
final long maxOrd = execution == ExecutionMode.GLOBAL_ORDINALS ? getMaxOrd(valuesSource, context.searcher()) : -1;
137137
if (execution == null) {
138138
execution = ExecutionMode.GLOBAL_ORDINALS;
139139
}

0 commit comments

Comments
 (0)