Add blockloader to patterned_text to support ESQL #134093
Conversation
parkertimmins
added
>non-issue
:StorageEngine/Mapping
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
| query: 'FROM test | SORT @timestamp | KEEP message, message.template_id | LIMIT 10' | ||
|
|
||
| - match: {columns.0.name: "message"} | ||
| - match: {columns.0.type: "text"} |
There was a problem hiding this comment.
Should this be updated to be patterned_text? This comes from the familyType, which is text. So I think leaving as text is correct 🤔
| } | ||
| } | ||
|
|
||
| public static class BytesRefsFromSimpleBinary extends AbstractBytesRefsFromBinary { |
There was a problem hiding this comment.
The existing BytesRefsFromBinary reader assumes some structure within the ByteRefs provided by the BinaryDocValues. Specifically, a number of value and some length and offsets for each value. The BinaryDocValues created for patterned_text has no such structure: the returned BytesRef contains a single value, which is the message string.
There was a problem hiding this comment.
As far as I can tell, BytesRefsFromBinary is expecting the bytes read out of the BinaryDocValues to be encoded in the format used by BinaryFieldMapper.CustomBinaryDocValuesField.
Maybe add comments to both BytesRefsFromBinary and BytesRefsFromSimpleBinary explaining which format each expects?
There was a problem hiding this comment.
Ahh, that's where it's from, thanks for tracking it down! Yeah, definitely makes sense to document how they are encoded. Thanks for the review!
There was a problem hiding this comment.
And then let's just rename this BytesRefsFromBinary?
| } | ||
| } | ||
|
|
||
| public static class BytesRefsFromSimpleBinary extends AbstractBytesRefsFromBinary { |
There was a problem hiding this comment.
As far as I can tell, BytesRefsFromBinary is expecting the bytes read out of the BinaryDocValues to be encoded in the format used by BinaryFieldMapper.CustomBinaryDocValuesField.
Maybe add comments to both BytesRefsFromBinary and BytesRefsFromSimpleBinary explaining which format each expects?
| /** | ||
| * Read BinaryDocValues encoded by {@link BinaryFieldMapper.CustomBinaryDocValuesField} | ||
| */ | ||
| static class BytesRefsFromBinary extends AbstractBytesRefsFromBinary { |
There was a problem hiding this comment.
Maybe rename this BytesRefsFromBinary to BytesRefsFromCustomBinary? Given that it only works with bytes created by CustomBinaryDocValuesField?
| } | ||
| } | ||
|
|
||
| public static class BytesRefsFromSimpleBinary extends AbstractBytesRefsFromBinary { |
There was a problem hiding this comment.
And then let's just rename this BytesRefsFromBinary?
4 participants
The existing blockloader for
patterned_textdoes not work correctly. Update it so thatpatterned_textworks in ESQL.