PerFieldPostingsFormat's "no suffix for format name" check misses equal formats with different names once another field used the second name: the commit succeeds and a field's postings cannot be read

PerFieldPostingsFormat lets a codec choose a postings format per field. Fields whose formats compare equal (PostingsFormat.equals) share one group of files, named after the format and a per-name number (EqA_0, EqB_0, ...). Each field records two attributes, its own format name (getName()) and the matched group's number, and the reader opens the field from the files <name>_<number>. Two equal formats with different names would therefore write a field into one group while its attributes point to another. Lucene guards this with an always-on check (IllegalStateException: no suffix for format name, PerFieldPostingsFormat.java:240-243 at 5adff78bb168), but the check asks whether any group used the field's name, not whether the matched group was created under it. With fields a -> EqB(2), x -> EqA(1), y -> EqB(1) (equal by setting only), y joins x's group, the check passes because a used EqB, and y records a's group: the commit succeeds, docFreq(y:v) is 0 and CheckIndex is clean. That silence needs the recorded (name, number) to name an existing group (here both numbers are 0); with other group numbers the commit also succeeds but the index then fails to open (CorruptIndexException). With only x and y the same mapping is rejected. It needs two SPI-registered formats with different names whose equals treats them as equal; no format shipped in Lucene overrides equals. Every release from 4.0.0 to 10.6.0 has it.

FieldsWriter.buildFieldsGroupMapping (:199-260) is shared by flush (:143) and merge (:181). It visits fields in name order (flush: index/FreqProxFields.java:38, "fields are already sorted by field name").

1. Groups are found by equals. formatToGroupBuilders is a HashMap<PostingsFormat, ...> (:202); formatToGroupBuilders.get(format) (:219) uses the format's equals and hashCode.
2. Numbers are counted per name. On a miss, the next number for format.getName() comes from suffixes (:205, :224-230), and the new group writes files <name>_<number> (:232-237).
3. The only check on a hit is if (!suffixes.containsKey(formatName)) throw new IllegalStateException("no suffix for format name: ...") (:240-243): has any group used this name? This is the fix site.
4. The field records its own name and the matched group's number: PER_FIELD_FORMAT_KEY = formatName, PER_FIELD_SUFFIX_KEY = groupBuilder.suffix (:217, :248-253).
5. The reader trusts those attributes: PostingsFormat.forName(formatName) and segment suffix formatName + "_" + suffix (:303-318).

In the repro, a creates group EqB_0, x creates EqA_0, and y's EqB(1) equals EqA(1), so y joins EqA_0 and its postings are written there. The check passes because a already used EqB, and y records (EqB, 0), which is a's group. The reader opens EqB_0 for y; it holds only a's terms, so terms(y) is null. The independent reproducer's probe printed exactly this (formats named ProbeA/ProbeB): files _0_ProbeA_0.* and _0_ProbeB_0.*; attributes a {ProbeB, 0}, x {ProbeA, 0}, y {ProbeB, 0}; terms(y) absent; CheckIndex at level 3 clean. Without field a, the probe's commit throws IllegalStateException: no suffix for format name: ProbeB, expected: 0.

On a group hit the matched group was created under some name N, so suffixes always holds N; the check can fail only when the field's name differs from N. So the only mappings it can reject are equal formats with different names, and it misses the case where another group used the field's name.

Other group numbers. The field records its own name with the matched group's number, so what a reader sees depends on whether that group exists. A fact-check variant on main maps a -> VarB(2), w -> VarA(7), x -> VarA(1), y -> VarB(1) (the same equals): y joins x's group VarA_1, the check passes because of a, and y records (VarB, 1), a group that was never written. The commit succeeds; DirectoryReader.open then throws CorruptIndexException: Problem reading index. (resource=_0_VarB_1.psm), caused by NoSuchFileException, and CheckIndex (level 3) reports clean=false, with assertions on and off. With fix A that commit is rejected: IllegalStateException: no suffix for format name: VarB, expected: 1.

• Who. Applications with their own per-field postings mapping (a PerFieldPostingsFormat subclass or a codec's getPostingsFormatForField hook) that use two of their own SPI-registered PostingsFormat classes with different names whose equals treats an instance of one as equal to an instance of the other. A third field whose format has the second name, and is not equal, must sort before the colliding field.
• Not hit. The default codec and every format shipped in Lucene: none overrides equals, so distinct instances never share a group. Mappings in which equal formats also have equal names.
• Effect. The commit succeeds and one field's postings cannot be read (they are written into another group's files). In the repro's layout this is silent with assertions on and off: queries on the field match nothing and CheckIndex is clean. With other group numbers it is loud but late: the index fails to open after the commit. With only the two colliding fields the same mistake is rejected at commit.
• Default vs opt-in. Opt-in custom codec code on @lucene.experimental extension points.
• Scale and likelihood. One document with three one-term fields; deterministic. Rated contrived: the recorded searches found no public report of anyone hitting even the existing two-field check.

TestPerFieldEqualFormatNames (one method, extends LuceneTestCase, package org.apache.lucene.codecs.perfield of the lucene/core tests, Apache license header) plus a two-line SPI file that registers its formats EqA and EqB (the reader looks formats up by name). In the commands, $REPRO is this finding's repro folder, which holds the SPI file at its top level. It is the hunt's one-test repro with the license header added and one comment line wrapped; with them it passes checkGoogleJavaFormat. The test draws no random values; it was run with seed DEADBEEF, and earlier tests of the same mapping also failed with seeds 1234ABCD, 5EED0E01, 5EED0D02, A0D17E01 and A0D17E02. With fix A it passes unchanged, assertions on and off.

#  Licensed to the Apache Software Foundation (ASF) under one or more
#  contributor license agreements.  See the NOTICE file distributed with
#  this work for additional information regarding copyright ownership.
#  The ASF licenses this file to You under the Apache License, Version 2.0
#  (the "License"); you may not use this file except in compliance with
#  the License.  You may obtain a copy of the License at
#
#       http://www.apache.org/licenses/LICENSE-2.0
#
#  Unless required by applicable law or agreed to in writing, software
#  distributed under the License is distributed on an "AS IS" BASIS,
#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
#  See the License for the specific language governing permissions and
#  limitations under the License.

org.apache.lucene.codecs.perfield.TestPerFieldEqualFormatNames$EqA
org.apache.lucene.codecs.perfield.TestPerFieldEqualFormatNames$EqB

cp $REPRO/src/org/apache/lucene/codecs/perfield/TestPerFieldEqualFormatNames.java $LUCENE/lucene/core/src/test/org/apache/lucene/codecs/perfield/
cp $REPRO/org.apache.lucene.codecs.PostingsFormat $LUCENE/lucene/core/src/test/META-INF/services/
cd $LUCENE && ./gradlew :lucene:core:test --tests org.apache.lucene.codecs.perfield.TestPerFieldEqualFormatNames -Ptests.seed=DEADBEEF

# gradle, scratch worktree at 5adff78bb168, -Ptests.seed=DEADBEEF (assertions on; -Ptests.asserts=false gives the same)
TestPerFieldEqualFormatNames > testEqualFormatsWithDifferentNames FAILED
    java.lang.AssertionError: expected:<1> but was:<0>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:EAAB3C0285D31F89]:0)
        ...
        at org.apache.lucene.codecs.perfield.TestPerFieldEqualFormatNames.testEqualFormatsWithDifferentNames(TestPerFieldEqualFormatNames.java:120)

# release jars 10.6.0 and 9.12.3, assertions off: the same AssertionError

# with fix A, assertions on and off
:lucene:core:test (SUCCESS): 1 test
# the commit now throws the existing check; the hunt's earlier test (same mapping, formats FormatA/FormatB) shows the message:
java.lang.IllegalStateException: no suffix for format name: FormatB, expected: 0

• The code already treats the mapping as illegal. The check at PerFieldPostingsFormat.java:240-243 can fail only when a field joins a group created under another name (see the mechanism), and it does reject the two-field mapping with IllegalStateException.
• Contract. The group map became equals-keyed on purpose in LUCENE-4090 (#5162); its author (Robert Muir) wrote there that equals on postings formats is "optional and a new optimization". LUCENE-4093 (#5165, still open) asks for such overrides. No javadoc of PostingsFormat, NamedSPI or the per-field hooks says equal formats must have equal names.
• Tests. TestPerFieldPostingsFormat2.testSameCodecDifferentInstance and testSameCodecDifferentParams (:262, :279) use same-name instances only; no test combines an equals override with two names.
• History. LUCENE-5969 (#7031, 98e9122375c) turned the original assert suffixes.containsKey(formatName) into the always-on exception with the same condition.

The strongest case for "not a bug" (rated moderate by the cross-model reviews and the devil's-advocate verifier): an equals that equates formats whose SPI names differ is broken user code, because the name is what gets written (the private name field is documented as "Unique name that's used to retrieve this format when reading the index", PostingsFormat.java:66); the extension points are @lucene.experimental; no shipped format overrides equals; and LUCENE-5969 turned every assert in the file into a check at once, so the always-on check was not a separate decision about this input.

Why it fails: no contract states the rule; Lucene already rejects this exact mapping, just incompletely; the commit succeeds instead of throwing, and the field is lost (silently with a clean CheckIndex in the repro's layout, or with an index that fails to open for other group numbers); and the fix leaves every valid mapping unchanged. The argument bears on likelihood (hence priority Low), not on correctness.

Option A was built and checked through gradle on 5adff78bb168; B and C were not run. A keeps today's behaviour for every mapping Lucene accepts now and turns the silent case into the rejection Lucene already gives for two fields. Side remark, not claimed: PerFieldDocValuesFormat (:191, :229) and PerFieldKnnVectorsFormat (:157, :179) use the same equals-keyed lookup with an assert-only guard; for doc values the two-field mapping fails loudly (an AssertionError at flush with assertions on; with them off, a commit after which DirectoryReader.open throws FileNotFoundException). Vectors were only read.

In buildFieldsGroupMapping, keep a map from each group's format to the name it was created with, and on a hit compare it with format.getName() instead of asking suffixes.containsKey(formatName). Same exception and message as today's two-field rejection.

      Map<PostingsFormat, String> groupNames = new HashMap<>();
      ...
          formatToGroupBuilders.put(format, groupBuilder);
          groupNames.put(format, formatName);
        } else {
          // we've already seen this format, so just grab its suffix
          if (!formatName.equals(groupNames.get(format))) {
            throw new IllegalStateException(
                "no suffix for format name: " + formatName + ", expected: " + groupBuilder.suffix);
          }

Trade-offs

Still a commit-time rejection, as today for two fields, not an error when the codec is configured. The message reads oddly for this case; a final version could name the field and both format names (not run). One extra HashMap per flush or merge, sized by the number of groups. Nothing rejected today becomes accepted: whenever the old check throws (no group used the field's name), the matched group was created under another name, so the new check throws too. Valid mappings are unchanged because the new check throws only on a hit whose group was created under another name. Merges apply the current mapping to the merged field names (PerFieldPostingsFormat.java:171-181), so with the fix, merging segments that an affected version wrote with such a mapping throws the same exception instead of carrying the lost field forward, and new flushes with that mapping are rejected too: the application has to fix its equals (read, not run).

What the checks validated

• Gradle in a scratch worktree at 5adff78bb168, seed DEADBEEF: the repro fails without the fix (assertions on and off) and passes with it (on and off).
• With the fix: the repro, TestPerFieldPostingsFormat (30), TestPerFieldPostingsFormat2 (6), TestPerFieldDocValuesFormat (150), TestPerFieldKnnVectorsFormat (61), TestCodecs (3) and TestAddIndexes (39): 290 tests, 0 failures, 16 skipped. :lucene:core:checkGoogleJavaFormat passes with the fix and the repro in the tree. The whole core module was not run.
• Earlier, the hunt's judge ran the same change through a shadowed class: the hunt's first test is then rejected at commit with IllegalStateException: no suffix for format name: FormatB, expected: 0, and its history version is rejected the same way on main.
• A fact-check variant with other group numbers (a -> VarB(2), w -> VarA(7), x -> VarA(1), y -> VarB(1)), assertions off: with the fix the commit throws IllegalStateException: no suffix for format name: VarB, expected: 1.

Use a composite key (format name, format) for the groups, so equal formats with different names get separate groups, each written under its own name. The mapping then works instead of being rejected.

Trade-offs

Changes today's two-field behaviour from rejection to success, and touches the Map<PostingsFormat, FieldsGroup> that write and merge iterate (PerFieldPostingsFormat.java:143-195). A bigger change than the bug needs; not built.

State the rule where per-field formats compare formats with equals.

Trade-offs

Does not stop the silent loss; worth adding together with A. Not built.

Method: semi-formal (a proof-obligation audit of a class's contract, witnessed by an executable repro).

• The obligation: a per-field postings mapping that IndexWriter accepts either writes a segment in which every field's postings can be read back, or is rejected. The audit covered PerFieldPostingsFormat.java:202-205, :217-254 (group lookup by equals, numbering per name, the guard, the attributes) and the reader at :300-320. The guard discharges the obligation only when no other group used the field's format name.
• A public-API JUnit test then failed on main with assertions on and off and on 10.6.0. The independent reproducer's probe (kept) printed the files, the attributes and a clean CheckIndex.
• An independent reproducer, two executed cross-model reviews, a devil's-advocate verifier and an adjudication reproduced it with their own tests ("Prior reports, history and checks").