englefly commented on code in PR #49851:
URL: https://github.com/apache/doris/pull/49851#discussion_r2055456954


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExtractSingleTableExpressionFromDisjunction.java:
##########
@@ -137,6 +142,23 @@ private Optional<Expression> 
extractSingleTableExpression(Expression expr, Strin
         for (Expression conjunct : conjuncts) {
             if (isSingleTableExpression(conjunct, qualifier)) {
                 output.add(conjunct);
+            } else if (conjunct instanceof Or) {
+                List<Expression> disjuncts = 
ExpressionUtils.extractDisjunction(conjunct);
+                List<Expression> extracted = 
Lists.newArrayListWithExpectedSize(disjuncts.size());
+                boolean success = true;
+                for (Expression disjunct : disjuncts) {
+                    Optional<Expression> extractedDisjunct = 
extractSingleTableExpression(disjunct, qualifier);
+                    if (extractedDisjunct.isPresent()) {
+                        extracted.add(extractedDisjunct.get());
+                    } else {
+                        // extract failed
+                        success = false;
+                        break;
+                    }
+                }
+                if (success) {
+                    output.add(ExpressionUtils.or(extracted));

Review Comment:
   how to avoid to add redandent expressions?
   suppose there are 3 layers in expr tree.
   `[(expr1 or expr2) and (expr3 or expr4)] or [part1] or [part2]`
   we extract `expr5` from `(expr1 or expr2)`, but cannot extract expression 
from part1 and part2.



##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ExtractSingleTableExpressionFromDisjunctionTest.java:
##########
@@ -215,4 +218,51 @@ private boolean 
verifySingleTableExpression4(List<Expression> conjuncts) {
         );
         return conjuncts.size() == 2 && conjuncts.contains(or);
     }
+
+    @Test
+    public void testExtract5() {

Review Comment:
   it is better to rename to "testExtractRecursive"



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to