Ghost fields and postings/points [LUCENE-10357]

[asfimport]

On doc values and norms, we require that codec APIs return non-null empty instances on fields that have the feature turned on on their FieldInfo, even if all values have been merged away.

However postings and points have the choice: they may either return empty instances or null. See e.g. BasePostingsFormatTestCase#testGhosts for instance.

I fear that this could be a source of bugs, as a caller could be tempted to assume that he would get non-null terms on a FieldInfo that has IndexOptions that are not NONE. Should we introduce a contract that FieldsProducer (resp. PointsReader) must return a non-null instance when postings (resp. points) are indexed?

---

Migrated from LUCENE-10357 by Adrien Grand (@jpountz), updated Jul 13 2022
Pull requests: #907

[javanna]

I've just seen an occurrence of this, FieldExistsQuery#count checks if getPointDimensionCount is greather than 0, and it may happen that such condition is true and the following getPointValues throws a NullPointerException.

[javanna]

@jpountz I am looking into fixing the points issue that I mentioned above. We could for instance ensure that Lucene90PointsReader never returns null and instead return an empty instance. On the other hand it looks like many callers of this method already check for null, and I see that getValues also throws exception in case FieldInfo#getPointDimensionCount is 0, which means that callers can't blindly call getValues without consulting FieldInfo first. I wonder then, should we fix FieldExistsQuery to handle the null return value, or adapt all other existing callers to not handle null? I think the former is quicker, the latter is cleaner, especially if like you said above "codec APIs are required to return non-null empty instances on fields that have the feature turned on on their FieldInfo, even if all values have been merged away."

[jpountz]

I see that getValues also throws exception in case FieldInfo#getPointDimensionCount is 0, which means that callers can't blindly call getValues without consulting FieldInfo first

It's a bit more complicated than that. Callers indeed cannot call PointsReader#getValues blindly, which is a codec API that should only be called on fields that have points enabled. However, callers can call LeafReader#getPointValues blindly, the user-facing API, which internally checks whether the field is indexed with points to know whether it should forward to the PointsReader#getValues codec API or return null. Queries are expected to always interact with points through LeafReader#getPointValues, not PointsReader#getValues. If we changed PointsReader#getValues to never return null, LeafReader#getPointValues would still return null on fields that do not exist or that do not have points enabled.

[jpountz]

I had hoped that getting rid of ghost fields would automatically help avoid some bugs but after looking into it for both postings and points (thanks @javanna and @shahrs87 !) it looks like it's not actually possible to make things significantly better. Hence closing this issue.