search

mikemccand / stargazers-migration-test

Testing Lucene's Jira -> GitHub issues migration
0 stars 0 forks source link

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

Open mikemccand opened 5 years ago

mikemccand 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?

Legacy Jira details

LUCENE-8924 by Atri Sharma (@atris) on Jul 17 2019

mikemccand commented 5 years ago

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

[Legacy Jira: Adrien Grand (@jpountz) on Jul 17 2019]

mikemccand commented 5 years ago

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)

[Legacy Jira: Atri Sharma (@atris) on Jul 17 2019]