github-actions[bot] commented on code in PR #64849:
URL: https://github.com/apache/doris/pull/64849#discussion_r3642340609
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateGroupByKey.java:
##########
@@ -110,17 +250,24 @@ public static Set<Expression>
findCanBeRemovedExpressions(LogicalAggregate<? ext
FuncDeps funcDeps = dataTrait.getAllValidFuncDeps(validSlots);
if (funcDeps.isEmpty()) {
- return new HashSet<>();
+ return new FindResult(new HashSet<>(), new HashSet<>());
}
Set<Set<Slot>> minGroupBySlots = funcDeps.eliminateDeps(new
HashSet<>(groupBySlots.values()), requireOutput);
Set<Expression> removeExpression = new HashSet<>();
+ Set<Expression> wrapWithAnyValue = new HashSet<>();
for (Entry<Expression, Set<Slot>> entry : groupBySlots.entrySet()) {
- if (!minGroupBySlots.contains(entry.getValue())
- && !requireOutput.containsAll(entry.getValue())) {
- removeExpression.add(entry.getKey());
+ if (!minGroupBySlots.contains(entry.getValue())) {
+ // FD redundant: can remove from group-by
+ if (!requireOutput.containsAll(entry.getValue())) {
+ // Not needed in output either: remove completely
+ removeExpression.add(entry.getKey());
+ } else {
+ // Still needed in output: remove from group-by, wrap with
ANY_VALUE in output
Review Comment:
**[P1] Do not trust nullable-side outer-join FDs for this rewrite**
`dataTrait.getAllValidFuncDeps` can include an FD copied from the nullable
side of an outer join even though null extension invalidates it. For example, a
right `Filter(b = 100)` derives `c -> b`; after a left join with one matched
row `(b=100,c=NULL)` and one unmatched row `(b=NULL,c=NULL)`, `c -> b` is
false. For `GROUP BY b,c` with `coalesce(b,-1)` above, this branch removes `b`
and wraps it in `ANY_VALUE`, merging the two groups into one. Previously the
output requirement kept `b` grouped. Please fence nullable-side join FDs before
using them here and cover matched plus unmatched rows with a nullable
determinant.
--
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]
