Skip to content

Do not enable doc-partitioning for count#143544

Merged
dnhatn merged 6 commits intoelastic:mainfrom
dnhatn:count-doc-partitioning
Mar 4, 2026
Merged

Do not enable doc-partitioning for count#143544
dnhatn merged 6 commits intoelastic:mainfrom
dnhatn:count-doc-partitioning

Conversation

@dnhatn
Copy link
Copy Markdown
Member

@dnhatn dnhatn commented Mar 4, 2026

We can't use Weight#count with doc-partitioning because if the query gets cached mid-way, the count becomes incorrect. Over-counting occurs when some parts are counted doc-by-doc while the first part is counted entirely via the cached shortcut. Under-counting occurs when the first part is not cached but later parts are, causing those parts to be skipped. To make doc-partitioning work properly for count, we would need coordination between threads.

This change also restores the default partitioning to SEGMENT, which accidentally changed to SHARD.

Closes #134512

@dnhatn dnhatn added :Analytics/ES|QL AKA ESQL >bug auto-backport Automatically create backport pull requests when merged v8.19.13 v9.3.2 v9.2.7 labels Mar 4, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @dnhatn, I've created a changelog YAML for you.

@elastic elastic deleted a comment from elasticmachine Mar 4, 2026
@dnhatn dnhatn requested review from nik9000 and romseygeek March 4, 2026 05:25
@dnhatn dnhatn marked this pull request as ready for review March 4, 2026 05:25
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Mar 4, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elastic elastic deleted a comment from elasticmachine Mar 4, 2026
Copy link
Copy Markdown
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

static LuceneSliceQueue.PartitioningStrategy partitioningStrategyForCount(Query q) {
final Query unwrapped = LuceneSourceOperator.Factory.unwrapQuery(q);
return switch (unwrapped) {
case MatchAllDocsQuery unused -> LuceneSliceQueue.PartitioningStrategy.SHARD;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: do we need the unused token here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we need it for switch.

@dnhatn
Copy link
Copy Markdown
Member Author

dnhatn commented Mar 4, 2026

Thanks Alan!

@dnhatn dnhatn merged commit a55d55c into elastic:main Mar 4, 2026
35 of 36 checks passed
@dnhatn dnhatn deleted the count-doc-partitioning branch March 4, 2026 18:26
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
9.3 Commit could not be cherrypicked due to conflicts
8.19 Commit could not be cherrypicked due to conflicts
9.2 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 143544

burqen pushed a commit to burqen/elasticsearch that referenced this pull request Mar 5, 2026
We can't use Weight#count with doc-partitioning because if the query 
gets cached mid-way, the count becomes incorrect. Over-counting occurs
when some parts are counted doc-by-doc while the first part is counted
entirely via the cached shortcut. Under-counting occurs when the first
part is not cached but later parts are, causing those parts to be
skipped. To make doc-partitioning work properly for count, we would need
coordination between threads.

This change also restores the default partitioning to SEGMENT, which 
accidentally changed to SHARD.

Closes elastic#134512
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 5, 2026
We can't use Weight#count with doc-partitioning because if the query 
gets cached mid-way, the count becomes incorrect. Over-counting occurs
when some parts are counted doc-by-doc while the first part is counted
entirely via the cached shortcut. Under-counting occurs when the first
part is not cached but later parts are, causing those parts to be
skipped. To make doc-partitioning work properly for count, we would need
coordination between threads.

This change also restores the default partitioning to SEGMENT, which 
accidentally changed to SHARD.

Closes elastic#134512
spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Mar 6, 2026
We can't use Weight#count with doc-partitioning because if the query 
gets cached mid-way, the count becomes incorrect. Over-counting occurs
when some parts are counted doc-by-doc while the first part is counted
entirely via the cached shortcut. Under-counting occurs when the first
part is not cached but later parts are, causing those parts to be
skipped. To make doc-partitioning work properly for count, we would need
coordination between threads.

This change also restores the default partitioning to SEGMENT, which 
accidentally changed to SHARD.

Closes elastic#134512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged backport pending >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.13 v9.2.7 v9.3.2 v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] CanMatchIT testAliasFilters failing

3 participants