Skip to content

branch-4.2: [fix](external) Keep sub-field predicate access paths #68331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hubgeter wants to merge 1 commit into
base: branch-4.2
Choose a base branch
from
+52 −20
Open

branch-4.2: [fix](external) Keep sub-field predicate access paths #68331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 35 additions & 20 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ && containsDataSkippingOnlyAccessPath(collectAccessPathResults)) {
buildColumnAccessPaths(slot, predicateAccessPaths);
AccessPathInfo accessPathInfo = result.get(slot.getExprId().asInt());
if (accessPathInfo != null) {
retainPredicatePathsInFinalAllAccessPaths(
alignPredicatePathsWithFinalAllAccessPaths(
predicatePaths, accessPathInfo.getAllAccessPaths());
accessPathInfo.getPredicateAccessPaths().addAll(predicatePaths);
}
Expand All @@ -413,7 +413,7 @@ && containsDataSkippingOnlyAccessPath(collectAccessPathResults)) {
buildColumnAccessPaths(slot, predicateAccessPaths);
AccessPathInfo accessPathInfo = result.get(slot.getExprId().asInt());
if (accessPathInfo != null) {
retainPredicatePathsInFinalAllAccessPaths(
alignPredicatePathsWithFinalAllAccessPaths(
predicatePaths, accessPathInfo.getAllAccessPaths());
accessPathInfo.getPredicateAccessPaths().addAll(predicatePaths);
}
Expand Down Expand Up @@ -861,34 +861,49 @@ private static void stripNullSuffixPaths(
}

/**
* Keep predicate access paths as a subset of final all access paths after NULL/OFFSET cleanup.
* Predicate paths are built from filter expressions first, but later all-path rewrites may drop
* metadata-only paths or collapse paths to whole-column access. Any predicate path not present
* in final all paths must be removed before sending access info to BE.
* Reconcile predicate access paths with the final all access paths. Predicate paths are built
* from filter expressions first, but later all-path rewrites drop redundant paths or collapse
* them to whole-column access, so a predicate path can end up outside the final all paths.
*
* <p>Examples:
* <ul>
* <li>All paths {@code [s]}, predicate paths {@code [s.city.NULL]} becomes no predicate
* paths after parent NULL removal.</li>
* <li>All paths {@code [s.city.NULL, s.zip]}, predicate paths
* {@code [s.NULL, s.city.NULL]} becomes {@code [s.city.NULL]}.</li>
* </ul>
* <p>A NULL/OFFSET path is dropped when it is no longer one of the all paths: BE switches the
* whole iterator to NULL_MAP_ONLY/OFFSET_ONLY when it sees such a path and skips the children,
* so it must not come back through the predicate paths either.
*
* <p>Any other path is kept, because BE needs it to read the predicate columns first and
* lazily materialize the rest. It is added to the all paths unless a wider path already covers
* it, e.g. the whole-column path {@code [s]} covers the predicate path {@code [s.city]}.
*/
private static void retainPredicatePathsInFinalAllAccessPaths(
private static void alignPredicatePathsWithFinalAllAccessPaths(
List<TColumnAccessPath> predicatePaths, List<TColumnAccessPath> allPaths) {
if (predicatePaths.isEmpty()) {
return;
}

List<TColumnAccessPath> toRemove = new ArrayList<>();
for (TColumnAccessPath predicatePath : predicatePaths) {
if (!allPaths.contains(predicatePath)) {
toRemove.add(predicatePath);
if (isMetaOnlyAccessPath(predicatePath)) {
if (!allPaths.contains(predicatePath)) {
toRemove.add(predicatePath);
}
} else if (!isCoveredByAllPath(predicatePath, allPaths)) {

@github-actions github-actions Bot Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not forward an unescaped literal * struct field

A struct field can legitimately be named * (for example from an external schema, and ordinal element_at copies that real name). If the query also outputs the whole struct, all paths collapse to [s], but this branch leaves predicate path [s, *] because the root covers it. FileScannerV2 parses predicate paths independently, and AccessPathParser::build_struct_children_from_access_node explicitly rejects * under STRUCT as a reserved array/map traversal token, so a query that previously read the full struct and evaluated the filter now fails during scanner setup. Please preserve/escape segment provenance or suppress unsupported covered subpaths, and add a file-scan regression for this field name.

allPaths.add(predicatePath);
}
}
predicatePaths.removeAll(toRemove);
}

private static boolean isMetaOnlyAccessPath(TColumnAccessPath accessPath) {
return accessPath.getType() == TAccessPathType.META
|| isDataSkippingOnlyAccessPath(getAccessPathList(accessPath));

@github-actions github-actions Bot Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve literal Variant keys named NULL/OFFSET

This classifies metadata solely from the final path string, but Variant string keys are copied verbatim. A valid predicate such as v['NULL'] = 1 or v['OFFSET'] = 1 therefore produces an ordinary DATA path ending in this token. When the query also projects the whole v, the final all path is [v] and this branch drops the predicate leaf; on file scans, normalizeDataSkippingOnlyAccessPaths similarly truncates it to [v] first. The scan then loses the independent leaf projection/lazy split that this PR restores for ordinary keys. Please preserve synthetic-suffix provenance (or otherwise make this classification path-origin/type aware) and cover both literal keys.

}

private static boolean isCoveredByAllPath(
TColumnAccessPath predicatePath, List<TColumnAccessPath> allPaths) {
for (TColumnAccessPath allPath : allPaths) {
if (allPath.getType() == predicatePath.getType()
&& pathCoversPrefix(getAccessPathList(allPath), getAccessPathList(predicatePath))) {

@github-actions github-actions Bot Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Normalize the root before testing prefix coverage

Complex access paths are collected with a lower-cased slot root, but a collapsed whole-column path is rebuilt from the catalog name. For an OLAP complex column named S, projecting S while filtering S.city therefore reaches this check as all path [S] and predicate path [s, city]; this case-sensitive comparison treats the leaf as uncovered and appends it to the all paths. BE accepts both roots case-insensitively, removes [S] as the whole-root marker, then uses the remaining city all-subpath to mark every sibling SKIP_READING, so the projected whole struct can silently omit/default-fill sibling values. Please canonicalize the root consistently (or compare it case-insensitively) and add an uppercase OLAP complex-column regression that asserts sibling values.

return true;
}
}
return false;
}

private static boolean hasStrictPrefix(List<String> path, List<String> prefix) {
return path.size() > prefix.size() && path.subList(0, prefix.size()).equals(prefix);
}
Expand Down
17 changes: 17 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,23 @@ public void testMapElementArrayNullPathCoveredByValueFieldAccess() throws Except
Assertions.assertFalse(predicateAccessPaths.contains(path("s", "m", "*", "NULL")));
}

@Test
public void testWholeColumnOutputKeepsSubFieldPredicatePath() throws Exception {
// The whole column is read, so all access paths collapse to the root path. The predicate
// path must survive, otherwise BE cannot read the predicate field first and lazily
// materialize the rest of the column.
Pair<PhysicalPlan, List<SlotDescriptor>> result = collectComplexSlots(
"select s from tbl where struct_element(s, 'city') = 'x'");
TreeSet<TColumnAccessPath> allAccessPaths = new TreeSet<>();
TreeSet<TColumnAccessPath> predicateAccessPaths = new TreeSet<>();
for (SlotDescriptor slotDescriptor : result.second) {
allAccessPaths.addAll(slotDescriptor.getAllAccessPaths());
predicateAccessPaths.addAll(slotDescriptor.getPredicateAccessPaths());
}
Assertions.assertEquals(ImmutableList.of(path("s")), ImmutableList.copyOf(allAccessPaths));
Assertions.assertEquals(ImmutableList.of(path("s", "city")), ImmutableList.copyOf(predicateAccessPaths));
}

@Test
public void testVariantAccessPath() throws Exception {
assertColumn("select v['a']['B'] from variant_tbl",
Expand Down
Loading