From 9fd11277424a9c055cffe1b3d40fdd79c64a7a0c Mon Sep 17 00:00:00 2001 From: daidai Date: Mon, 21 Sep 2026 17:31:00 +0800 Subject: [PATCH] branch-4.2: [fix](nested column pruning) Keep sub-field predicate access paths #68214 added retainPredicatePathsInFinalAllAccessPaths, which removes every predicate access path that is not literally one of the final all access paths. That is right for NULL/OFFSET paths, which are stripped from the all paths on purpose, but it also removes ordinary sub-field paths whenever the all paths collapse to the whole-column path: SELECT s FROM tbl WHERE struct_element(s, 'city') = 'x' -- all: [s], predicate: [] (was [s.city]) BE then cannot tell which sub-column the predicate reads, loses the eager/lazy split and reads the column as one unit. On branch-4.2 this shows up in test_iceberg_variant_read as FilteredRowsByLazyRead = 0 for SELECT CAST(v AS STRING) FROM variant_page_pruning WHERE CAST(v['n'] AS INT) > 3000 Only file scans hit this: on OLAP tables a variant sub-path predicate gets its own sub-column slot, which is why the existing tests did not catch it. Keep the NULL/OFFSET cleanup, since BE switches the whole iterator to NULL_MAP_ONLY/OFFSET_ONLY once such a path shows up. Any other predicate path is kept, and added to the all paths when no wider path covers it, as master's addPredicatePathsToFinalAllAccessPaths does. --- .../rules/rewrite/NestedColumnPruning.java | 55 ++++++++++++------- .../rules/rewrite/PruneNestedColumnTest.java | 17 ++++++ 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java index 8056ebd747c214..d60686c2ec6030 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java @@ -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); } @@ -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); } @@ -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. * - *

Examples: - *

+ *

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. + * + *

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 predicatePaths, List allPaths) { - if (predicatePaths.isEmpty()) { - return; - } - List 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)) { + allPaths.add(predicatePath); } } predicatePaths.removeAll(toRemove); } + private static boolean isMetaOnlyAccessPath(TColumnAccessPath accessPath) { + return accessPath.getType() == TAccessPathType.META + || isDataSkippingOnlyAccessPath(getAccessPathList(accessPath)); + } + + private static boolean isCoveredByAllPath( + TColumnAccessPath predicatePath, List allPaths) { + for (TColumnAccessPath allPath : allPaths) { + if (allPath.getType() == predicatePath.getType() + && pathCoversPrefix(getAccessPathList(allPath), getAccessPathList(predicatePath))) { + return true; + } + } + return false; + } + private static boolean hasStrictPrefix(List path, List prefix) { return path.size() > prefix.size() && path.subList(0, prefix.size()).equals(prefix); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java index cd0e08327b29cc..dfe5f595d9e3b1 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java @@ -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> result = collectComplexSlots( + "select s from tbl where struct_element(s, 'city') = 'x'"); + TreeSet allAccessPaths = new TreeSet<>(); + TreeSet 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",