FrankChen021 commented on code in PR #20149:
URL: https://github.com/apache/druid/pull/20149#discussion_r3886675882
##########
processing/src/main/java/org/apache/druid/segment/VirtualColumns.java:
##########
@@ -282,7 +282,14 @@ public VirtualColumn findEquivalent(Node otherNode)
toCheckForEquivalence =
otherVirtualColumn.rewriteRequiredColumns(equivalenceRewriteMap);
}
- return equivalence.get().get(toCheckForEquivalence.getEquivalanceKey());
+ VirtualColumn matched =
equivalence.get().get(toCheckForEquivalence.getEquivalanceKey());
+ if (matched != null &&
+ // guardrail check for expression collision when a virtual column
shadows the physical column
+ // e.x. otherNode v0 = dim1 and VCs dim1 = dim2 plus q = dim1, q can
be treated as equivalent to v0 even though it reads physical column dim2
+ getNode(matched.getOutputName()).getDependencies().size() ==
otherNode.getDependencies().size()) {
Review Comment:
P1 Virtual-column dependency counting misses physical-name collisions
The dependency count can conflate a query virtual column with a physical
column that shadows one of its dependencies. For example, with physical `a=x`
and `v0=concat(a,b)`, and query virtual columns `b=x` and `q=concat(b,b)`, the
rewritten expressions can look identical even though query `b` shadows physical
`b`. The cluster-group pruner can then treat `q` as `v0` and discard segments
that contain matching rows. Compare resolved bindings or reject rewrite
collisions, and add a nested-shadow regression test.
--
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]
