You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/02 09:42:42 UTC

[GitHub] [incubator-doris] qzsee opened a new pull request, #9942: [Enhancement] (Nereids) scalar expression rewrite framework

qzsee opened a new pull request, #9942:
URL: https://github.com/apache/incubator-doris/pull/9942

   # Proposed changes
   
   Issue Number: close #9633
   
   ## Problem Summary:
   
   Describe the overview of changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   2. Has unit tests been added: (Yes/No/No Need)
   3. Has document been added or modified: (Yes/No/No Need)
   4. Does it need to update dependencies: (Yes/No)
   5. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] jackwener commented on pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#issuecomment-1147582720

   Please correct style according to [doc](https://doris.apache.org/zh-CN/developer-guide/java-format-code.html)


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] qzsee commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
qzsee commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r889815227


##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -173,6 +174,10 @@ comparisonOperator
     : EQ | NEQ | LT | LTE | GT | GTE | NSEQ
     ;
 
+logicalOperator

Review Comment:
   for test . forget to delete 



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#issuecomment-1154797145

   PR approved by anyone and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] jackwener commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r890285520


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/EqualTo.java:
##########
@@ -17,8 +17,13 @@
 
 package org.apache.doris.nereids.trees.expressions;
 
+import com.google.common.base.Preconditions;
 import org.apache.doris.nereids.exceptions.UnboundException;
 import org.apache.doris.nereids.trees.NodeType;
+import org.apache.doris.nereids.trees.TreeNode;
+
+import java.util.List;
+import java.util.Objects;

Review Comment:
   redundant



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] qzsee commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
qzsee commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r894390127


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewriter.java:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite;
+
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeExpressionRule;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.SimplifyNotExprRule;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.LeafExpression;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Expression rewrite entry, which contains all rewrite rules.
+ */
+public class ExpressionRewriter {

Review Comment:
   you are right . I fixed it



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] EmmyMiao87 merged pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 merged PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] github-actions[bot] commented on pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#issuecomment-1154797108

   PR approved by at least one committer and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] jackwener commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r890277954


##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewriteTest.java:
##########
@@ -0,0 +1,84 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite;
+
+import org.apache.doris.catalog.Database;
+import org.apache.doris.nereids.parser.SqlParser;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeExpressionRule;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NotExpressionRule;
+import org.apache.doris.nereids.trees.expressions.Expression;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ExpressionRewriteTest {
+    private static final Logger LOG = LogManager.getLogger(Database.class);
+
+    private static final SqlParser PARSER = new SqlParser();
+    private ExpressionRewriter rewriter;
+
+    @Test
+    public void testNotExpressionRewrite() {
+        rewriter = new ExpressionRewriter(NotExpressionRule.INSTANCE);
+
+        assertRewrite("NOT 'X'", "NOT 'X'");
+        assertRewrite("NOT NOT 'X'", "'X'");
+        assertRewrite("NOT NOT NOT 'X'", "NOT 'X'");
+        assertRewrite("NOT 'X' > 'Y'", "'X' <= 'Y'");
+        assertRewrite("NOT 'X' < 'Y'", "'X' >= 'Y'");
+        assertRewrite("NOT 'X' >= 'Y'", "'X' < 'Y'");
+        assertRewrite("NOT 'X' <= 'Y'", "'X' > 'Y'");
+        assertRewrite("NOT 'X' = 'Y'", "NOT 'X' = 'Y'");
+        assertRewrite("NOT NOT 'X' > 'Y'", "'X' > 'Y'");
+        assertRewrite("NOT NOT NOT 'X' > 'Y'", "'X' <= 'Y'");
+        assertRewrite("NOT NOT NOT 'X' >  NOT NOT 'Y'", "'X' <= 'Y'");
+        assertRewrite("NOT 'X' > NOT NOT 'Y'", "'X' <= 'Y'");
+
+    }
+
+    @Test
+    public void testNormalizeExpressionRewrite() {
+        rewriter = new ExpressionRewriter(NormalizeExpressionRule.INSTANCE);
+
+        assertRewrite("2 > 'x'", "'x' < 2");
+        assertRewrite("2 >= 'x'", "'x' <= 2");
+        assertRewrite("2 < 'x'", "'x' > 2");
+        assertRewrite("2 <= 'x'", "'x' >= 2");
+        assertRewrite("2 = 'x'", "'x' = 2");
+        /*
+        assertRewrite("'a' = 1", "'a' = 1");
+        assertRewrite("'a' = 1 and 1 = 'a'", "'a' = 1");
+        assertRewrite("'a' = 1 and 'b' > 2 and 'a' = 1", "'a' = 1 and 'b' > 2");
+        assertRewrite("'a' = 1 and 'a' = 1 and 'b' > 2 and 'a' = 1 and 'a' = 1", "'a' = 1 and 'b' > 2");
+
+        assertRewrite("'a' = 1 or 'a' = 1", "'a' = 1");
+        assertRewrite("'a' = 1 or 'a' = 1 or 'b' >= 1", "'a' = 1 or 'b' >= 1");
+        */
+    }
+
+    private void assertRewrite(String expression, String expected) {
+        Expression expectedExpression = PARSER.createExpression(expected);
+        Expression needRewriteExpression = PARSER.createExpression(expression);
+        Expression rewrittenExpression = rewriter.rewrite(needRewriteExpression);
+        LOG.info("original expression : {} expected expression : {} rewritten expression : {}", needRewriteExpression, expectedExpression, rewrittenExpression);

Review Comment:
   Remove the `LOG` in UT?



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] morrySnow commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r894323373


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/RewriteHelper.java:
##########
@@ -0,0 +1,57 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite;
+
+import org.apache.doris.nereids.analyzer.UnboundSlot;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.Literal;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Expression rewrite helper class.
+ */
+public class RewriteHelper {
+
+    public static boolean isConstant(Expression expr) {
+        return expr.isConstant();
+    }
+
+    public static Expression convertLiteralToSlot(Expression expr) {
+        ExpressionRewriter rewriter = new ExpressionRewriter(ConvertLiteralToSlotRule.INSTANCE);
+        return rewriter.rewrite(expr);
+    }
+
+    /**
+     * For test.
+     * convert literal to slot

Review Comment:
   if just for test, it's better move it to test folder



##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewriteTest.java:
##########
@@ -0,0 +1,79 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite;
+
+import org.apache.doris.catalog.Database;
+import org.apache.doris.nereids.parser.SqlParser;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeExpressionRule;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.SimplifyNotExprRule;
+import org.apache.doris.nereids.trees.expressions.Expression;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * all expr rewrite rule test case.
+ */
+public class ExpressionRewriteTest {
+    private static final Logger LOG = LogManager.getLogger(Database.class);
+
+    private static final SqlParser PARSER = new SqlParser();
+    private ExpressionRewriter rewriter;
+
+    @Test
+    public void testNotExpressionRewrite() {
+        rewriter = new ExpressionRewriter(SimplifyNotExprRule.INSTANCE);
+
+        assertRewrite("NOT x > y", "x <= y");
+        assertRewrite("NOT x < y", "x >= y");
+        assertRewrite("NOT x >= y", "x < y");
+        assertRewrite("NOT x <= y", "x > y");
+        assertRewrite("NOT x = y", "NOT x = y");
+        assertRewrite("NOT NOT x > y", "x > y");
+        assertRewrite("NOT NOT NOT x > y", "x <= y");
+    }
+
+    @Test
+    public void testNormalizeExpressionRewrite() {
+        rewriter = new ExpressionRewriter(NormalizeExpressionRule.INSTANCE);
+
+        assertRewrite("2 > x", "x < 2");
+        assertRewrite("2 >= x", "x <= 2");
+        assertRewrite("2 < x", "x > 2");
+        assertRewrite("2 <= x", "x >= 2");
+        assertRewrite("2 = x", "x = 2");
+        /*

Review Comment:
   remove it if unused



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] morrySnow commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r891019837


##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -141,7 +141,8 @@ expression
     ;
 
 booleanExpression
-    : valueExpression                                              #predicated
+    : valueExpression                                                                        #predicated
+    | NOT booleanExpression                                                                  #not

Review Comment:
   need at first place



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] wangshuo128 commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r894345674


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewriter.java:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite;
+
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeExpressionRule;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.SimplifyNotExprRule;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.LeafExpression;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Expression rewrite entry, which contains all rewrite rules.
+ */
+public class ExpressionRewriter {

Review Comment:
   I'm afraid this is duplicated with `ExpressionVisitor`.
   We can traverse the expression tree in the `applyRule` or by the visitor, it's not a good design.



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] jackwener commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r890735114


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/NotExpressionRule.java:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite.rules;
+
+import org.apache.doris.nereids.rules.expression.rewrite.AbstractExpressionRewriteRule;
+import org.apache.doris.nereids.rules.expression.rewrite.ExpressionRewriteContext;
+import org.apache.doris.nereids.trees.NodeType;
+import org.apache.doris.nereids.trees.expressions.ComparisonPredicate;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.GreaterThan;
+import org.apache.doris.nereids.trees.expressions.GreaterThanEqual;
+import org.apache.doris.nereids.trees.expressions.LessThan;
+import org.apache.doris.nereids.trees.expressions.LessThanEqual;
+import org.apache.doris.nereids.trees.expressions.Literal;
+import org.apache.doris.nereids.trees.expressions.Not;
+
+public class NotExpressionRule extends AbstractExpressionRewriteRule {

Review Comment:
   `SimplifyNotExprRule`



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/NormalizeExpressionRule.java:
##########
@@ -0,0 +1,59 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite.rules;
+
+import org.apache.doris.nereids.rules.expression.rewrite.AbstractExpressionRewriteRule;
+import org.apache.doris.nereids.rules.expression.rewrite.ExpressionRewriteContext;
+import org.apache.doris.nereids.rules.expression.rewrite.RewriteHelper;
+import org.apache.doris.nereids.trees.NodeType;
+import org.apache.doris.nereids.trees.expressions.ComparisonPredicate;
+import org.apache.doris.nereids.trees.expressions.EqualTo;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.GreaterThan;
+import org.apache.doris.nereids.trees.expressions.GreaterThanEqual;
+import org.apache.doris.nereids.trees.expressions.LessThan;
+import org.apache.doris.nereids.trees.expressions.LessThanEqual;
+
+
+public class NormalizeExpressionRule extends AbstractExpressionRewriteRule {

Review Comment:
   Rename `AdjustCmpExprOrderRule`.
   
   normalize is so general.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/NormalizeExpressionRule.java:
##########
@@ -0,0 +1,59 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite.rules;
+
+import org.apache.doris.nereids.rules.expression.rewrite.AbstractExpressionRewriteRule;
+import org.apache.doris.nereids.rules.expression.rewrite.ExpressionRewriteContext;
+import org.apache.doris.nereids.rules.expression.rewrite.RewriteHelper;
+import org.apache.doris.nereids.trees.NodeType;
+import org.apache.doris.nereids.trees.expressions.ComparisonPredicate;
+import org.apache.doris.nereids.trees.expressions.EqualTo;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.GreaterThan;
+import org.apache.doris.nereids.trees.expressions.GreaterThanEqual;
+import org.apache.doris.nereids.trees.expressions.LessThan;
+import org.apache.doris.nereids.trees.expressions.LessThanEqual;
+
+

Review Comment:
   Add example in header javadoc



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [incubator-doris] morrySnow commented on a diff in pull request #9942: [Enhancement] (Nereids) scalar expression rewrite framework

Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #9942:
URL: https://github.com/apache/incubator-doris/pull/9942#discussion_r889790374


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Literal.java:
##########
@@ -91,11 +97,41 @@ public boolean nullable() throws UnboundException {
         return value == null;
     }
 
+    @Override
+    public boolean isVariable() {
+        // for test

Review Comment:
   test code?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java:
##########
@@ -61,4 +66,13 @@ public Expression child(int index) {
     public EXPR_TYPE newChildren(List<TreeNode> children) {
         throw new RuntimeException();
     }
+
+    public boolean isVariable() {

Review Comment:
   please add comment to this function



##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -147,6 +147,7 @@ booleanExpression
 valueExpression
     : primaryExpression                                                                      #valueExpressionDefault
     | left=valueExpression comparisonOperator right=valueExpression                          #comparison
+    | NOT valueExpression                                                                    #not

Review Comment:
   not should be placed under booleanExpression as 'NOT booleanExpression'



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewriter.java:
##########
@@ -0,0 +1,73 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite;
+
+import com.google.common.collect.Lists;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeExpressionRule;
+import org.apache.doris.nereids.rules.expression.rewrite.rules.NotExpressionRule;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.LeafExpression;
+
+import java.util.List;
+
+public class ExpressionRewriter {
+
+    public static final List<ExpressionRewriteRule> REWRITE_RULES = Lists.newArrayList(
+        new NotExpressionRule(),
+        new NormalizeExpressionRule()
+    );
+
+    private final ExpressionRewriteContext ctx;
+    private final List<ExpressionRewriteRule> rules;
+
+    public ExpressionRewriter(List<ExpressionRewriteRule> rules) {
+        this.rules = rules;
+        this.ctx = new ExpressionRewriteContext();
+    }
+
+    public ExpressionRewriter(ExpressionRewriteRule rule) {
+        this.rules = Lists.newArrayList(rule);
+        this.ctx = new ExpressionRewriteContext();
+    }
+
+
+    public Expression rewrite(Expression root) {
+        Expression result = root;
+        for (ExpressionRewriteRule rule : rules) {
+            result = applyRule(result, rule);
+        }
+        return result;
+    }
+
+    private Expression applyRule(Expression expr, ExpressionRewriteRule rule) {
+        Expression rewrittenExpr = expr;
+        rewrittenExpr = applyRuleBottomUp(rewrittenExpr, rule);
+        return rewrittenExpr;
+    }
+
+    private Expression applyRuleBottomUp(Expression expr, ExpressionRewriteRule rule) {
+        List<Expression> children = Lists.newArrayList();

Review Comment:
   add a todo: apply function could return a flag  indicating whether expression has changed. We could not replace children if no child has changed.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/rules/NotExpressionRule.java:
##########
@@ -0,0 +1,74 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.rules.expression.rewrite.rules;
+
+import org.apache.doris.nereids.rules.expression.rewrite.AbstractExpressionRewriteRule;
+import org.apache.doris.nereids.rules.expression.rewrite.ExpressionRewriteContext;
+import org.apache.doris.nereids.trees.NodeType;
+import org.apache.doris.nereids.trees.expressions.ComparisonPredicate;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.GreaterThan;
+import org.apache.doris.nereids.trees.expressions.GreaterThanEqual;
+import org.apache.doris.nereids.trees.expressions.LessThan;
+import org.apache.doris.nereids.trees.expressions.LessThanEqual;
+import org.apache.doris.nereids.trees.expressions.Literal;
+import org.apache.doris.nereids.trees.expressions.Not;
+
+public class NotExpressionRule extends AbstractExpressionRewriteRule {
+
+    public static NotExpressionRule INSTANCE = new NotExpressionRule();
+
+
+    @Override
+    public Expression visitNotExpression(Not expr, ExpressionRewriteContext context) {
+
+        Expression child = expr.child();
+
+        if (child instanceof ComparisonPredicate) {
+            ComparisonPredicate cp = (ComparisonPredicate) expr.child();
+            Expression left =  cp.left();
+            Expression right = cp.right();
+            NodeType type = cp.getType();
+            switch (type) {
+                case GREATER_THAN:
+                    return new LessThanEqual(left, right);
+                case GREATER_THAN_EQUAL:
+                    return new LessThan(left, right);
+                case LESS_THAN:
+                    return new GreaterThanEqual(left, right);
+                case LESS_THAN_EQUAL:
+                    return new GreaterThan(left, right);
+                default:
+                    return expr;
+            }
+        }
+
+        if (child instanceof Not) {
+            Not son = (Not) child;
+            return son.child();
+        }
+
+        return expr;
+    }
+
+    @Override
+    public Expression visitLiteral(Literal expr, ExpressionRewriteContext context) {

Review Comment:
   useless code



##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -173,6 +174,10 @@ comparisonOperator
     : EQ | NEQ | LT | LTE | GT | GTE | NSEQ
     ;
 
+logicalOperator

Review Comment:
   not use anymore?



-- 
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-unsubscribe@doris.apache.org

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


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