Add maxValueCount to DocValuesSkipper metadata by iprithv · Pull Request #15993 · apache/lucene

iprithv · 2026-04-28T21:31:46Z

Description

resolves #15794

Add DocValuesSkipper#maxValueCount() so consumers can determine whether a doc values field is definitely single-valued without unwrapping iterators.
Persist exact global maxValueCount metadata in Lucene90 doc values skipper metadata for new segments.
Add validation and test coverage for empty, single-valued, and multi-valued skipper fields.

Compatibility

Older Lucene90 segments do not have this metadata, so maxValueCount() returns a safe upper bound: 0 for empty fields and Integer.MAX_VALUE otherwise. Exact values become available once segments are rewritten or merged with the new format.

jainankitk

I am wondering if this information needs to stored as part of DocValuesSkipper as this should be single value per field per segment right? Also, this PR - #15737 might be related.

iprithv · 2026-05-01T17:40:21Z

I am wondering if this information needs to stored as part of DocValuesSkipper as this should be single value per field per segment right? Also, this PR - #15737 might be related.

Thanks for taking a look.

Yes, maxValueCount is a single global value per field per segment. This PR stores it that way: it is part of the Lucene90 DocValuesSkipperEntry field metadata, alongside the existing global minValue, maxValue, docCount, and maxDocId. It is not stored per skip interval/block.

I put it on DocValuesSkipper because this is where Lucene already exposes doc-values-derived segment-level metadata for fields that have a skip index. Putting it in FieldInfo would make it look like schema/static field metadata, but it is actually a computed per-segment doc values statistic and may be unavailable when no skipper is present.

iprithv · 2026-05-09T17:52:56Z

@romseygeek would you be able to take a look at this? Thanks!

msfroh

This looks reasonable to me.

My only concern is whether it addresses @romseygeek's sentence on #15794:

This is fragile, and not necessarily supported by Codecs.

This still depends on the individual DocValuesFormat implementation storing and returning this data.

I don't know how we can reasonably implement it in a DocValuesSkipper if we don't add it at the DocValuesFormat layer, though. (That might be a gap in my understanding, though.)

iprithv · 2026-05-14T22:06:26Z

@msfroh yes right, this does lean on the DocValuesFormat to actually store the number. But docCount, minValue, and maxValue on the skipper already have that same dependency. The default maxValueCount() returns Integer.MAX_VALUE when the format doesn't know the exact count, so it's a safe fallback. It's just a conservative upper bound.

I thought about FieldInfo, but that feels like schema metadata and this is really a per-segment stat. Sticking it on the iterator felt wrong too. That's for walking docs, not for segment-level stats. And computing it on demand is exactly the fragile thing @romseygeek was pointing out in #15794.

Yeah, fields without a skipper are out of luck here. But those are usually not the numeric/ordinal fields the issue is about. For fields that do have a skipper, this kills the weird hack where we unwrap SortedNumericDocValues to see if it's secretly a singleton. Thanks!

msfroh · 2026-05-14T23:28:01Z

For fields that do have a skipper, this kills the weird hack where we unwrap SortedNumericDocValues to see if it's secretly a singleton.

Right! From an understandability standpoint, I think this is a big win. (Also, I'm imagining some potential efficiency gains that one could eke out when they know that max value count is "low".)

iprithv · 2026-05-18T02:57:43Z

@jpountz would you be able to take a look at this? thanks!

romseygeek · 2026-05-18T09:13:53Z

+    if (version >= Lucene90DocValuesFormat.VERSION_SKIPPER_MAX_VALUE_COUNT) {
+      maxValueCount = meta.readInt();
+    } else {
+      maxValueCount = docCount == 0 ? 0 : Integer.MAX_VALUE;


I think we can be a bit smarter here? eg, for Numeric/Sorted entries we know this value is 1, and for SortedNumeric/SortedSet we can check the relevant XXEntry and see if numValues == numDocsWithField and return 1 for those cases?

I think I'd prefer returning -1 to returning Integer.MAX_VALUE for cases where we don't know the exact value to make it clear that the data isn't available.

makes sense, sure. done. Thanks!

romseygeek · 2026-05-18T11:37:31Z


        readFields(in, state.fieldInfos);

+        // For older segments without maxValueCount metadata, infer it from entry data


Let's pull this into a separate method and gate it behind a version check

done, thanks!

romseygeek · 2026-05-19T08:10:04Z

Thanks for all the iterations @iprithv, this is looking great. There's a lint failure in there, can you fix that up?

iprithv · 2026-05-20T00:36:10Z

Thanks for all the iterations @iprithv, this is looking great. There's a lint failure in there, can you fix that up?

sure, done. Thanks for reviewing this!

Returns the maximum document cardinality for docs in a segment. For older indexes where this information is not available, returns -1. Singleton instances will return 1 even on older indexes.

romseygeek · 2026-05-20T11:11:49Z

Thanks @iprithv - I adjusted the CHANGES entry and milestone to be for 10.5 as I don't think this needs to wait for a major release.

Port inferMaxValueCounts from apache/lucene#15993 to the TSDB codec so segments written before VERSION_SKIPPER_MAX_VALUE_COUNT return 1 rather than -1 when the field type or per field metadata proves the field is single valued. Applied in AbstractTSDBDocValuesProducer for ES95 and ES819, and in ES87TSDBDocValuesProducer for the legacy codec.

Port the consumer and producer changes from apache/lucene#15993 to the TSDB doc values codec so the new DocValuesSkipper.maxValueCount() API returns accurate values rather than the unknown sentinel. The newer ES95 and ES819 codecs persist and read maxValueCount via the shared abstract base classes, with a TSDB format version bump to VERSION_SKIPPER_MAX_VALUE_COUNT to gate adoption. The legacy ES87 codec, which throws on writes in production but is still exercised by tests through a writable consumer in the test source tree, receives parallel changes scoped to its own files and its own format version. Closes elastic#149715

Port inferMaxValueCounts from apache/lucene#15993 to the TSDB codec so segments written before VERSION_SKIPPER_MAX_VALUE_COUNT return 1 rather than -1 when the field type or per field metadata proves the field is single valued. Applied in AbstractTSDBDocValuesProducer for ES95 and ES819, and in ES87TSDBDocValuesProducer for the legacy codec.

…9778) * fix(tsdb): wire DocValuesSkipper.maxValueCount() Port the consumer and producer changes from apache/lucene#15993 to the TSDB doc values codec so the new DocValuesSkipper.maxValueCount() API returns accurate values rather than the unknown sentinel. The newer ES95 and ES819 codecs persist and read maxValueCount via the shared abstract base classes, with a TSDB format version bump to VERSION_SKIPPER_MAX_VALUE_COUNT to gate adoption. The legacy ES87 codec, which throws on writes in production but is still exercised by tests through a writable consumer in the test source tree, receives parallel changes scoped to its own files and its own format version. Closes #149715 * fix(tsdb): infer maxValueCount for old segments Port inferMaxValueCounts from apache/lucene#15993 to the TSDB codec so segments written before VERSION_SKIPPER_MAX_VALUE_COUNT return 1 rather than -1 when the field type or per field metadata proves the field is single valued. Applied in AbstractTSDBDocValuesProducer for ES95 and ES819, and in ES87TSDBDocValuesProducer for the legacy codec.

github-actions Bot added module:core/index module:core/search module:core/codecs module:test-framework labels Apr 28, 2026

github-actions Bot added this to the 11.0.0 milestone Apr 28, 2026

jainankitk reviewed Apr 30, 2026

View reviewed changes

iprithv requested a review from jainankitk May 1, 2026 17:42

msfroh approved these changes May 14, 2026

View reviewed changes

romseygeek reviewed May 18, 2026

View reviewed changes

iprithv force-pushed the dv-skipper-max-count branch 3 times, most recently from cc3340c to 5e76ffa Compare May 18, 2026 10:52

iprithv requested a review from romseygeek May 18, 2026 10:52

romseygeek requested changes May 18, 2026

View reviewed changes

iprithv force-pushed the dv-skipper-max-count branch from 5e76ffa to e86f86e Compare May 18, 2026 16:27

iprithv requested a review from romseygeek May 18, 2026 16:29

review changes

634c4d9

iprithv force-pushed the dv-skipper-max-count branch from e86f86e to 634c4d9 Compare May 20, 2026 00:18

lint error fix

e132476

romseygeek approved these changes May 20, 2026

View reviewed changes

romseygeek merged commit 696f4e7 into apache:main May 20, 2026
13 checks passed

romseygeek modified the milestones: 11.0.0, 10.5.0 May 20, 2026

romseygeek added a commit that referenced this pull request May 20, 2026

Adjust changes entry for #15993

8d7d259

This was referenced May 22, 2026

[CI] Missing wiring of DocValuesSkipper.maxValueCount() in TSDB codec elastic/elasticsearch#149715

Closed

fix(tsdb): wire DocValuesSkipper.maxValueCount() in TSDB codec elastic/elasticsearch#149778

Merged


		readFields(in, state.fieldInfos);

		// For older segments without maxValueCount metadata, infer it from entry data

Uh oh!

Conversation

iprithv commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Compatibility

Uh oh!

jainankitk left a comment

Choose a reason for hiding this comment

Uh oh!

iprithv commented May 1, 2026

Uh oh!

iprithv commented May 9, 2026

Uh oh!

msfroh left a comment

Choose a reason for hiding this comment

Uh oh!

iprithv commented May 14, 2026

Uh oh!

msfroh commented May 14, 2026

Uh oh!

iprithv commented May 18, 2026

Uh oh!

romseygeek May 18, 2026

Choose a reason for hiding this comment

Uh oh!

iprithv May 18, 2026

Choose a reason for hiding this comment

Uh oh!

romseygeek May 18, 2026

Choose a reason for hiding this comment

Uh oh!

iprithv May 18, 2026

Choose a reason for hiding this comment

Uh oh!

romseygeek commented May 19, 2026

Uh oh!

iprithv commented May 20, 2026

Uh oh!

Uh oh!

romseygeek commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iprithv commented Apr 28, 2026 •

edited

Loading