github-actions[bot] commented on code in PR #59263:
URL: https://github.com/apache/doris/pull/59263#discussion_r3488825491
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java:
##########
@@ -325,33 +331,75 @@ private static Map<Integer, AccessPathInfo> pruneDataType(
return result;
}
+ private static Pair<ColumnAccessPathType, List<String>>
normalizePredicateMetaPathForAllAccessPath(
+ CollectAccessPathResult accessPathResult, boolean
hasRegularAccessPath) {
+ List<String> path = accessPathResult.getPath();
+ ColumnAccessPathType pathType = accessPathResult.getType();
+ if (accessPathResult.isPredicate() && hasRegularAccessPath
Review Comment:
This strips the predicate metadata before the only gate that decides whether
map-star paths are expanded. In the new `cardinality(map_arr_struct_col['a']) >
0` case, collection produces a regular projection path like
`map_arr_struct_col.*.*.verified` and a predicate metadata path
`map_arr_struct_col.*.OFFSET`; because there is a regular path, this branch
inserts the stripped all-path `map_arr_struct_col.*` into the all-access tree.
That tree no longer has an OFFSET marker, so phase 1.5 skips
`expandMapStarPaths()`. The result can stay as wildcard/current-level value
access instead of the expected `KEYS` plus `VALUES.OFFSET`, and the stripped
`*` path also makes the map value array look fully accessed rather than just
the projected `verified` field. Please drive the expansion decision from the
original predicate metadata paths, or expand before stripping the metadata
suffix.
##########
regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy:
##########
@@ -122,9 +123,8 @@ suite("null_column_pruning") {
explain {
sql "select id, arr_col from ncp_tbl where arr_col is null"
contains "nested columns"
- contains "all access paths: [arr_col]"
- notContains "arr_col.NULL"
- notContains "predicate access paths:"
+ contains "all access paths: [arr_col, arr_col.NULL]"
Review Comment:
This assertion does not match the access paths the FE now constructs. When
the slot also has a regular data path,
`normalizePredicateMetaPathForAllAccessPath()` strips a predicate `NULL` path
to the data prefix before inserting it into `allAccessPaths`, and
`addPredicatePathsToFinalAllAccessPaths()` deliberately skips metadata
predicate paths. For this query the final all-path list collapses to
`[arr_col]`, while the predicate list keeps `[arr_col.NULL]`, so the new
`contains "all access paths: [arr_col, arr_col.NULL]"` check will fail. The
same mismatch appears in the new `map_col` and full `struct_col` checks below;
please either update the expected all paths to the stripped form or change the
FE logic to actually keep metadata in `allAccessPaths`.
--
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]
