apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.64k stars 1.02k forks source link

Remove Fields Order Checks from CheckIndex? [LUCENE-8924] #9967

Open asfimport opened 5 years ago

asfimport commented 5 years ago

CheckIndex checks the order of fields read from the FieldsEnum for the posting reader. Since we do not explicitly sort or use a sorted data structure to represent keys (atleast explicitly), and no FieldsEnum depends on the order apart from MultiFieldsEnum, which no longer exists.

 

Should we remove the check?


Migrated from LUCENE-8924 by Atri Sharma (@atris)

asfimport commented 5 years ago

Adrien Grand (@jpountz) (migrated from JIRA)

We rely on the order for merging, see "MultiFields".

asfimport commented 5 years ago

Atri Sharma (@atris) (migrated from JIRA)

I see. 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 (since Ser.copyOf explicitly calls out that there is no guarantee in the order of traversal)