Skip to content

[9.2] fix(esql): check per-shard DateFieldType for DocValuesSkipper (#142752)#142850

Merged
elasticsearchmachine merged 7 commits intoelastic:9.2from
salvatore-campagna:backport/9.2/pr-142752
Mar 3, 2026
Merged

[9.2] fix(esql): check per-shard DateFieldType for DocValuesSkipper (#142752)#142850
elasticsearchmachine merged 7 commits intoelastic:9.2from
salvatore-campagna:backport/9.2/pr-142752

Conversation

@salvatore-campagna
Copy link
Contributor

Backport

This will backport the following commits from main to 9.2:

Questions ?

Please refer to the Backport tool documentation

…lastic#142752)

hasDocValuesSkipper was derived once from the first shard's
DateFieldType and applied globally via doWithContexts. In mixed
environments (TSDB shards with DocValuesSkipper + standard shards
with PointValues), calling the wrong API on some shards caused
sentinel values (Long.MIN_VALUE/Long.MAX_VALUE) to leak into min/max.

Replace doWithContexts with direct per-context iteration so each
shard's own DateFieldType determines whether to use DocValuesSkipper
or PointValues. This also simplifies the early-return guard by
removing the incorrect indexed check that could bail out prematurely
in mixed modes.

(cherry picked from commit 56bfc12)
Long result = null;
try {
for (final SearchExecutionContext context : contexts) {
if (context.isFieldMapped(field.string()) == false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: indexType not available in version 9.2

@salvatore-campagna salvatore-campagna self-assigned this Feb 23, 2026
@salvatore-campagna salvatore-campagna added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Feb 23, 2026
salvatore-campagna and others added 5 commits February 23, 2026 20:53
On 9.2 the setting defaults to false, so the backported test assertion
on hasDocValuesSkipper() always failed. Make both values explicit and
add a separate test for TSDB without the skipper enabled.
@salvatore-campagna
Copy link
Contributor Author

The testDocValuesSkipperMinMaxDoesNotReturnSentinelValues test was failing because on 9.2 index.mapping.use_doc_values_skipper defaults to false (the feature is opt-in), unlike main where it's enabled by default. Fixed by explicitly setting the setting to true in the test. Also added a companion test that covers the same TSDB scenario with the skipper disabled, exercising the PointValues fallback path.

@elasticsearchmachine elasticsearchmachine merged commit 3623f1f into elastic:9.2 Mar 3, 2026
35 checks passed
@salvatore-campagna salvatore-campagna deleted the backport/9.2/pr-142752 branch March 3, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport v9.2.7

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants