GITHUB#9967: document the Fields#iterator() order contract and assert it in tests

[serhiy-bzhezytskyy]

Description

CheckIndex rejects a reader whose Fields#iterator() returns field names out of order, but the requirement is stated nowhere and checked nowhere else.

What relies on the order is MergedIterator, used by MultiFields#iterator and by PerFieldPostingsFormat#merge. It documents "the behavior is undefined if the iterators are not actually sorted" rather than checking, so an unsorted producer yields wrong results with no error anywhere. Measured, with removeDuplicates=true as both callers use:

input	result
[a,b] + [a,c] — sorted	[a, b, c] — the shared name is deduplicated
[b,a,c] + [a,z] — one unsorted	[a, b, a, c, z] — a twice, from two different sub-iterators

Meanwhile:

• Fields#iterator() said only "Returns an iterator that will step through all fields names".
• FieldsConsumer#write has a Notes list of what an implementation must do and may assume, and did not mention the order, although Lucene103BlockTreeTermsWriter relies on it and AssertingFieldsConsumer has asserted it since 2013.
• FieldsProducer, which every postings reader extends, had no mention either.
• The comment on the CheckIndex check cites MultiFieldsEnum, which no longer exists.

So a new PostingsFormat — a public extension point — can return HashMap#keySet(), pass the whole test suite, and only be caught later by CheckIndex, if anyone runs it.

All thirteen Fields implementations in the repository do honour the order today, but by four different means: a TreeMap in four of them, an explicit sort on the way out in two, and in FreqProxFields a LinkedHashMap plus a comment relying on the caller having sorted first.

Changes

• Fields#iterator(), FieldsConsumer#write and FieldsProducer state the requirement, and Fields#iterator() also says what breaks without it.
• AssertingLeafReader.AssertingFields asserts it. That class already wrapped the Fields that term vectors expose, so the check covers that path too; AssertingPostingsFormat calls the same helper rather than carrying a second copy.
• AssertingFieldsConsumer#write now wraps the incoming Fields, so a violation fails during the write that caused it rather than in a later reader. This is part of the TODO that has sat in that method since 2013 (017a3bf6281).
• The CheckIndex comment names MultiFields and MergedIterator.

Doing the TODO's last part literally does not work, which is worth recording: AssertingFieldsProducer wraps Terms in AssertingLeafReader.AssertingTerms, which encodes read-side expectations, and on the write side the consumer pulls a PostingsEnum straight out of the incoming Fields and drives it differently. Reusing the producer trips assert super.docID() == nextDoc in AssertingPostingsEnum and fails three tests in BasePostingsFormatTestCase. So the wrapper is split: one class checks only the iterator() contract and is safe on both sides, a subclass adds the Terms wrapping for the read side. The "limited CheckIndex" half of the TODO is left in place, unaddressed.

Relationship to #9967

That issue asks whether the order check can be removed from CheckIndex. It cannot: MultiFields still exists and still merges these iterators, so @jpountz's 2019 answer — "We rely on the order for merging, see MultiFields" — still holds. But the follow-up question in the same thread was never answered:

Should we make this more explicit and robust then? For E.g., since we do not explicitly maintain a sort order but rely on the key set to do the right thing, a change from Collections.unModifiableSet to Set.copyOf breaks this assertion in checkIndex

This is an attempt at that. The check stays; the contract behind it is now written down and enforced where a codec author will see it.

Verification

Every claim the javadoc makes is pinned by a test, since a documented invariant that no test exercises is how the stale MultiFieldsEnum comment survived for thirteen years:

claim	test
ascending order from a real reader	testCheckIndexAcceptsSortedFields
CheckIndex verifies it	testCheckIndexDetectsFieldsOutOfOrder
MultiFields merges with MergedIterator	testMultiFieldsMergesInOrder
PerFieldPostingsFormat does too	testPerFieldMergePreservesFieldsExactlyOnce
unsorted input stops deduplicating	testUnsortedInputBreaksDeduplication
the write side receives sorted names	testWriteSideReceivesSortedFields
the order is not the insertion order	testOrderIsIndependentOfInsertionOrder
the assertion fires, and only on violations	TestAssertingPostingsFormat, TestAssertingLeafReader

Each was mutation-checked: inverting the expectation, removing the CheckIndex check, bypassing the assertion, or reversing the names the recording consumer observes makes the corresponding test fail.

:lucene:core:test, :lucene:test-framework:test, :lucene:codecs:test, :lucene:backward-codecs:test, :lucene:memory:test pass (11,452 tests), as do :lucene:core:check and tidy. Also run with -Ptests.nightly=true -Ptests.iters=3 -Ptests.asserts=true on the postings tests.

One thing worth flagging

The assertion is in test-framework, so a third-party PostingsFormat that violates the contract will start failing its own tests. That is what the assertion is for, but it is a behavioural change for downstream, so it is worth a deliberate decision rather than slipping in.

I checked what is reachable: Solr has no Fields implementation of its own (SchemaCodecFactory only resolves format names through SPI). Of the implementations in Elasticsearch and OpenSearch, DelegatingBloomFilterFieldsProducer inherits its delegate's order, TSDBSyntheticIdFieldsProducer exposes a single field, and Lucene40BlockTreeTermsReader sorts explicitly. Two places have no guarantee either way — XPerFieldMergeState's new ArrayList<>(filterFields), and TermVectorsFields backed by a HashMap from StreamInput#readMap, though that one is on the term-vectors path. This is from reading their code, not from running their tests.