This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 9d243cd7e31 branch-4.2: [fix](external) Keep sub-field predicate
access paths (#68331)
9d243cd7e31 is described below
commit 9d243cd7e310f07d945119104f7844feb7d6caf5
Author: daidai <[email protected]>
AuthorDate: Tue Sep 22 10:26:10 2026 +0800
branch-4.2: [fix](external) Keep sub-field predicate access paths (#68331)
### What problem does this PR solve?
Issue Number: None
Related PR: #68214
Problem Summary:
`retainPredicatePathsInFinalAllAccessPaths`, added by #68214, removes
every predicate access path that is not literally one of the final all
access paths. That is what NULL/OFFSET paths need, because they 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:
```sql
SELECT s FROM tbl WHERE struct_element(s, 'city') = 'x';
-- all access paths: [s], predicate access paths: [] (was [s.city])
```
BE then cannot tell which sub-column the predicate reads, so it loses
the eager/lazy split and reads the column as one unit. On branch-4.2
this shows up in `test_iceberg_variant_read`, where the lazy-read check
sees `FilteredRowsByLazyRead = 0` for
```sql
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, so its all paths already contain the
predicate path. That is why the existing unit tests did not catch it,
and why the new test uses a struct column, which keeps one slot for the
whole column.
This PR keeps the NULL/OFFSET cleanup, since BE switches the whole
iterator to `NULL_MAP_ONLY`/`OFFSET_ONLY` once such a path shows up and
skips the children. Every other predicate path is kept, and added to the
all paths when no wider path covers it — the behaviour of master's
`addPredicatePathsToFinalAllAccessPaths`.
### Release note
Fix nested/variant sub-field predicates losing lazy materialization on
branch-4.2 when the whole column is read.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
- Behavior changed:
- [ ] No.
- [x] Yes. Sub-field predicates keep their access paths, restoring
predicate-first reads.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
---
.../nereids/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 8056ebd747c..d60686c2ec6 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 @@ public class NestedColumnPruning implements CustomRewriter {
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 @@ public class NestedColumnPruning implements CustomRewriter {
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 @@ public class NestedColumnPruning implements
CustomRewriter {
}
/**
- * 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)) {
+ 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<TColumnAccessPath> allPaths)
{
+ for (TColumnAccessPath allPath : allPaths) {
+ if (allPath.getType() == predicatePath.getType()
+ && pathCoversPrefix(getAccessPathList(allPath),
getAccessPathList(predicatePath))) {
+ 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);
}
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 71accf14247..f3aa4bf42de 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 class PruneNestedColumnTest extends
TestWithFeService implements MemoPatt
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",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
