airborne12 commented on code in PR #66877:
URL: https://github.com/apache/doris/pull/66877#discussion_r3843799775
##########
be/src/storage/index/snii/snii_index_reader.cpp:
##########
@@ -1094,6 +1096,13 @@ Status SniiIndexReader::_try_count_only_fastpath(
RETURN_IF_ERROR(
::doris::snii::query::count_only_term_df(*logical_reader,
physical_term, &count));
}
+ const auto& stats = logical_reader->stats();
+ if (count > stats.doc_count || count > stats.indexed_doc_count) {
Review Comment:
The mechanism is real and is now gated in d123e3ac64e. Your example,
however, does not reproduce it — correcting that below so the follow-up work
targets the right shape.
**Confirmed.** `ArrayColumnWriter::append_nullable`
(`column_writer.cpp:1149`) calls `append_data` first, which reaches
`add_array_values(..., offsets_ptr, num_rows)` for every row of the batch
(`:1115`); `offsets_ptr` comes from
`OlapColumnDataConvertorArray::convert_to_olap`
(`olap_data_convertor.cpp:922`), which reads the nested `ColumnArray` offsets
and never consults the outer null map; the `add_array_nulls` that follows only
records null row ids (`snii_index_writer.cpp:433`) and retracts nothing.
CLucene is identical (`InvertedIndexColumnWriter::add_array_nulls`,
`inverted_index_writer.cpp:296`, only touches `_null_bitmap`), so this is not a
SNII deviation and cannot be repaired reader-side. Your arithmetic is right
too: with `doc_count=2, indexed_doc_count=1, df(alpha)=1`, `df <=
indexed_doc_count` holds, fabrication moves the id off the null row, and COUNT
returns 1 where the decode path returns 0.
**Correction to the trigger.** `if(cond, array_value, NULL)` produces an
*empty* nested range on the null rows, so it does not create this shape.
`FunctionIf` computes `result_nested_column` from `get_nested_column(arg_else)`
(`if.cpp:396`), and for a NULL literal that is the const default — an empty
array. The shape is reachable through the default nullable implementation
instead: `PreparedFunctionImpl::default_implementation_for_nulls` documents
that nested columns keep "arbitrary values in rows corresponding to NULL value"
(`function.h:170`) and `need_replace_null_data_to_default()` is `false` by
default (`function.h:138`), so e.g. `array_concat(arr, nullable_arr)` writes
`arr`'s tokens on a row the null map marks NULL.
**Scalars are unaffected**, which is why the gate is ARRAY-only:
`ScalarColumnWriter::append_nullable` (`column_writer.cpp:656`) splits the
batch into runs and routes null runs to `append_nulls()`, which emits no tokens.
**What this PR does.** `SniiIndexReader` now knows whether the column is an
ARRAY (`ColumnReader::_load_index` already tests `_meta_type ==
OLAP_FIELD_TYPE_ARRAY` a few lines above the reader construction), and the fast
path declines an ARRAY column on any segment that has a null bitmap. Arrays on
segments with no nulls keep the shortcut — no null row can exist there, so `df`
is null-free by construction. New case
`CountFastPathDeclinesAnArrayColumnHoldingANullRowInAPosting` runs the same
segment bytes twice, declared ARRAY and declared scalar, and shows the two
answers diverging after `mask_out_null` (1 vs 2).
**Deliberately not in this PR:** "fix new writes". Teaching
`add_array_values` to skip outer-null rows is a behaviour change shared with
CLucene and touches index semantics; it belongs in its own PR rather than in a
reader-side hardening change. Recorded as follow-up.
--
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]
