Make index-entry-to-record conversion an explicit contract

[hatyo]

Three physical plans read an index directly and have to hand back a record rather than the raw index entry: the covering index plan, the aggregate index plan, and the Lucene spell-check plan. They shared the mechanism for doing so — IndexKeyValueToPartialRecord, reached through a static helper in QueryPlanUtils — but not the thing they actually differ on, which is the shape an entry is decoded into. The covering plan decodes into a partial copy of the base record, resolving the descriptor from the record type. The aggregate plan decodes into the result of its select-having, whose aggregate column exists in no stored record, so it resolves a descriptor from the TypeRepository and looks its record type up differently as well. Because of that it could not use the shared helper and carried an inlined copy of it instead. The difference between the two was visible only as a difference between two blocks of code that otherwise look alike, which is an easy thing to get wrong while every individual step reads correctly.

IndexEntryToRecordConversion states that contract. An implementor converts a single entry through indexEntryToQueriedRecord, and does so via one of two named static helpers: intoStoredRecordShape for a partial copy of the record type, or intoShape when the target is a shape no stored record has. The choice is now said out loud rather than inferred from which lookup a plan happens to use. The pieces are passed as arguments rather than bundled into an object, since conversion runs once per index entry read. Making the Lucene spell-check plan an implementor too leaves QueryPlanUtils holding nothing, so it is removed, and RecordQueryCoveringIndexPlan's function-returning overload goes with it — its one caller in ComposedBitmapIndexQueryPlan now converts per entry, still with an empty evaluation context and a comment explaining why that suffices.

No behaviour change is intended, so the existing suites are the safety net: FDBCoveringIndexQueryTest, BitmapValueIndexTest, GroupByTest, FDBPermutedMinMaxQueryTest and the LuceneIndexTest spell-check cases all pass (116 tests), along with checkstyle and PMD on both touched modules. There is no new unit test — the remaining seam needs a record store, so it is covered by those FDB-backed tests rather than in isolation. Two things worth a reviewer's attention: QueryPlanUtils was public with no @API annotation, so if the team treats that as API surface this wants the breaking change label rather than enhancement; and conversion now resolves its pieces per entry rather than once per cursor, which is a repeated meta-data lookup — and, for the Lucene plan, a rebuilt converter. Nothing is allocated to wrap them, and memoizing is left to implementors with a note on the interface that anything cached must be scoped to one execution, since a plan may run against more than one store or context.

[normen662]

Nice little improvement! One comment re: naming and/or trait conventions.

[normen662]

Can you name this something that makes it explicit it's a plan trait? So something ...Plan.... Also if I look at the existing class IndexKeyValueToPartialRecord, from the name I cannot discern which one does what.

Also, for plans I think we always make the trait extend RecordQueryPlan.

[hatyo]

done, there are some exceptions in existing mixin / traits such that they extend nothing, but we could fix that later.