github-actions[bot] commented on code in PR #66423:
URL: https://github.com/apache/doris/pull/66423#discussion_r3717605056
##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -915,8 +927,11 @@ static bool rewrite_struct_element_path_to_file_expr(
return false;
}
for (size_t idx = 0; idx < struct_element_chain.size(); ++idx) {
- if (!can_filter_before_table_nullability_alignment(
- resolved.file_child_types[idx],
struct_element_chain[idx]->data_type())) {
+ // Accessor results become nullable for missing ARRAY indices and NULL
parents. Compare the
+ // file child with the declared table child instead, or that
execution-only wrapper can
+ // hide a nullable-file-to-required-table contract violation before
alignment reports it.
+ if
(!can_filter_before_table_nullability_alignment(resolved.file_child_types[idx],
Review Comment:
[P1] Validate every child in an ARRAY's full projection
These checks cover only the selected accessor chain, but ARRAY indexing
requests `project_all_children`, so TableReader later aligns unreferenced
siblings too. For table `items ARRAY<STRUCT<a Nullable(INT), b INT NOT NULL>>`,
file `items ARRAY<STRUCT<a Nullable(INT), b Nullable(INT)>>`, rows
`[{a=0,b=NULL}]` and `[{a=20,b=1}]`, and `items[1].a > 5`, this guard sees
compatible ARRAY/element/`a` types and localizes the filter. The file reader
then discards the first row before required sibling `b` is aligned, changing
the schema error into successful output. Please validate every projected
descendant before localizing a full ARRAY projection (or keep the filter
table-level), and add an end-to-end sibling-nullability regression.
##########
be/src/exprs/vectorized_fn_call.cpp:
##########
@@ -671,11 +671,24 @@ bool VectorizedFnCall::is_deterministic() const {
}
bool VectorizedFnCall::is_safe_to_execute_on_selected_rows() const {
- static const std::set<std::string> TOTAL_PREDICATE_FUNCTIONS = {
- "eq", "ne", "lt", "le", "gt", "ge", "in", "not_in",
"is_null_pred", "is_not_null_pred"};
+ static const std::set<std::string> TOTAL_PREDICATE_FUNCTIONS = {"eq",
+
"eq_for_null",
+ "ne",
+ "lt",
+ "le",
+ "gt",
+ "ge",
+ "in",
+ "not_in",
+
"is_null_pred",
+
"is_not_null_pred",
+
"element_at",
Review Comment:
[P1] Fence constant pruning after a rejected accessor
Adding `element_at` here also keeps the table-side constant-pruning prefix
open. For table `items ARRAY<STRUCT<a INT NOT NULL>>, z INT DEFAULT 0`, a file
containing only `items ARRAY<STRUCT<a Nullable(INT)>>`, predicates `[items[1].a
> 10, z = 7]`, and a row with `a=NULL`, the mapper correctly leaves the first
predicate above TableReader so required-child alignment can report the schema
error. But missing `z` is a constant/default filter entry, so
`_evaluate_constant_filters()` skips the nonconstant first filter, evaluates
`z=7` as false, and closes the split before any row reaches alignment. Before
this accessor was classified safe, the first conjunct closed that prefix.
Please propagate a rejected mapping/materialization barrier into constant
pruning too, and add a missing/default-column regression for this order.
--
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]
