PatternTextCompositeValues refactoring#140939
Conversation
martijnvg
commented
Jan 19, 2026
martijnvg
commented
- Renamed PatternTextFallbackDocValues to PatternTextFallbackDocValues.
- Made PatternTextFallbackDocValues an abstract class
- Added two subclasses for the case when the fallback values are stored as stored fields and for when fallback values are stored as binary doc values.
* Renamed PatternTextFallbackDocValues to PatternTextFallbackDocValues. * Made PatternTextFallbackDocValues an abstract class * Added two subclasses for the case when the fallback values are stored as stored fields and for when fallback values are stored as binary doc values.
martijnvg
added
>refactoring
:StorageEngine/Mapping
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
Kubik42
left a comment
Kubik42
left a comment
There was a problem hiding this comment.
Looks good! Just some minor comments.
| return null; | ||
| } | ||
|
|
||
| var docValues = PatternTextDocValues.from( |
There was a problem hiding this comment.
[nit] I don't agree with the naming of this class. PatternTextFallbackDocValues implies the values are for fallback, but in this case, if there is no fallback, then we return PatternTextDocValues. Can we merge this from() method with the one in PatternTextDocValues?
There was a problem hiding this comment.
In advanceExact(), we first check whether patternTextDocValues has a value and if not we check either binary doc values or stored fields. So it can be seen as a fallback?
Can we merge this from() method with the one in PatternTextDocValues?
Yes, that makes sense.
| FieldInfo fieldInfo = leafReader.getFieldInfos().fieldInfo(fieldType.storedNamed()); | ||
| if (fieldInfo == null) { | ||
| // If there is no stored subfield (either binary doc values or stored field), | ||
| // then there is no need to use PatternTextCompositeValues |
There was a problem hiding this comment.
comment still mentions PatternTextCompositeValues although that class no longer exists
3 participants
