Doris-Breakwater commented on issue #69062: URL: https://github.com/apache/doris/issues/69062#issuecomment-6163634182
Breakwater-GitHub-Analysis-Slot: slot_d6e4eee01f4d The reported load failure is supported by the source code. This merits BE inverted-index ingestion triage: an unsupported index term propagates a flush error for the batch. The CLucene rejection is an intentional crash-prevention safeguard; the proposed change would add graceful handling in Doris. Please open the PR, with query correctness covered alongside successful ingestion. **Verified from source** - I checked the public `4.1.4` tag (`ad35a140c7fd`) and current upstream master (`d5aec29f763b`). The `4.1.4` CLucene submodule already points to `08d45e541fef`, the guard introduced by [doris-thirdparty#368](https://github.com/apache/doris-thirdparty/pull/368/files). It rejects a NUL followed by any later non-NUL character; an all-NUL suffix is allowed. This is not a reason to remove that safeguard. - Untokenized scalar values below `ignore_above` go through a length-bearing field value into `add_document()`. Arrays similarly pass each eligible element into the document. Neither path checks embedded NUL. `add_document()` closes the writer on a CLucene exception and returns `INVERTED_INDEX_CLUCENE_ERROR`. See the [4.1.4 writer](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/index/inverted/inverted_index_writer.cpp#L239-L509) and [current master writer](https://github.com/apache/doris/blob/d5aec29f763b96740eaa43d549e8b0309d200c82/be/src/storage/index/inverted/inverted_index_writer.cpp#L374-L440). - Column writing propagates that status, and the flush token records and returns the failure rather than filtering one input row. This matches the reported batch failure. See [column writing](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/segment/column_writer.cpp#L618-L627) and [flush error handling](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/load/memtable/memtable_flush_executor.cpp#L268-L280). Materialized VARIANT paths attach their sub-column indexes to ordinary column writers, so the reported shared-writer mechanism is consistent with the [VARIANT writer helpers](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/segment/variant/variant_writer_helpers.cpp#L185-L221). **Important gap in a writer-only fix** Keeping the stored bytes does **not** automatically make an index-assisted query scan the row. The existing `ignore_above` behavior has a matching query-side length check that returns `INVERTED_INDEX_EVALUATE_SKIPPED`. The [string reader](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/index/inverted/inverted_index_reader.cpp#L501-L551) has no equivalent embedded-NUL check, and [segment fallback](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/storage/segment/segment_iterator.cpp#L1291-L1325) depends on an explicit status. Omitting a term therefore introduces a correctness risk for predicates on that value; storage preservation alone does not establish the claimed scan behavior. For the skip approach, add a matching read-side fallback for unsupported exact lookup values, including array membership and VARIANT path predicates. Audit other index-backed MATCH/prefix/regexp predicates too: an ordinary pattern may match an omitted value even without containing NUL itself. String range predicates already have bypass checks in the inspected version; preserve and test those paths. These are source-based review concerns, not results from running the proposed patch. The write-side implementation should use the actual untokenized/analyzer decision and a length-bounded check matching CLucene's rule. Preserve one document and one row-id increment per scalar/array row, including arrays where every element is skipped. Use `add_null_document()` for an omitted scalar term, **not** `add_nulls()`: the latter also marks the row in the SQL null bitmap. Skipped non-null values must remain non-null. A skip counter and rate-limited warning are useful; define whether the count measures rows or elements. Escaping terms is a broader alternative requiring coordinated write/query encoding and compatibility with existing segments. **Information and validation needed next** - Share the PR and test results, exact FE/BE build commits and CLucene revision, and the Stream Load request headers plus response JSON. Include minimal ARRAY and VARIANT `field_pattern` DDL/payloads; the scalar example already provides enough detail for source triage. - Add BE tests for leading/interior NUL, `a\0\0b`, trailing/all-NUL suffixes, empty strings, SQL NULL, `ignore_above` boundaries, mixed/all-skipped arrays, and valid rows before and after the skipped value. Verify row-id alignment and analyzer behavior for the parsers actually tested. - Add SQL regression tests proving batch success and byte preservation (for example with `HEX`), index use for `s = 'ok'`, and identical results with index evaluation enabled/disabled for NUL equality, inequality, IN/NOT IN, null predicates, array membership, and VARIANT path predicates. Include applicable range/pattern predicates and checks after compaction/index rebuild. Writer unit tests alone cannot prove scan fallback. This assessment is based on source inspection and the supplied error/reproducer. I have not executed a Doris cluster reproduction or the proposed tests, and the patch is not yet available for review. No query profile is needed to establish this failure mechanism. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
