Use IndexType in TermBasedFieldType constructors #137906
Conversation
In preparation for doc-value skippers being enabled in some keyword field types, TermBasedFieldType should take IndexType directly in its constructor rather than booleans for indexed and docvalues.
romseygeek
added
>non-issue
:StorageEngine/Mapping
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
felixbarny
left a comment
felixbarny
left a comment
There was a problem hiding this comment.
LGTM, just an optional suggestion.
| KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate | ||
| ) { | ||
| super(name, true, false, false, tsi, meta, isSyntheticSource, withinMultiField); | ||
| super(name, IndexType.terms(true, false), false, tsi, meta, isSyntheticSource, withinMultiField); |
There was a problem hiding this comment.
IndexType.terms(true, false) appears 14 times in this PR and exists in 4 more places in the existing code. What do you think about adding IndexType.termsOnly() to make it a bit more expressive? We could even return an internal constant IndexType instead of creating an equal instance very time IndexType.termsOnly() is called.
There was a problem hiding this comment.
Hm, having agreed - I'm considering moving the stored field into IndexType as well, given that it's another lucene data structure, which I think would make the suggested change less useful. So I think I will leave it for a follow up investigation.
|
|
||
| public ParentIdFieldType(String name, boolean eagerGlobalOrdinals) { | ||
| super(name, true, false, true, TextSearchInfo.SIMPLE_MATCH_ONLY, Collections.emptyMap()); | ||
| super(name, IndexType.terms(true, true), false, TextSearchInfo.SIMPLE_MATCH_ONLY, Collections.emptyMap()); |
There was a problem hiding this comment.
Similar story here. Maybe replace IndexType.terms(true, true) with IndexType.termsAndDocValues()
3 participants
In preparation for doc-value skippers being enabled in some keyword
field types, TermBasedFieldType should take IndexType directly in its
constructor rather than booleans for indexed and docvalues.