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 2021/08/26 12:56:22 UTC
[GitHub] [incubator-doris] zbtzbtzbt opened a new pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to fold const
zbtzbtzbt opened a new pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518
## Proposed changes
Fold const in compoundpredicate 'OR' 'AND' to hit prefix index
Related PR:
- [5623](https://github.com/apache/incubator-doris/pull/5623)
- [6400](https://github.com/apache/incubator-doris/pull/6400)
- [3616](https://github.com/apache/incubator-doris/pull/3616)
## Types of changes
What types of changes does your code introduce to Doris?
_Put an `x` in the boxes that apply_
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation Update (if none of the other choices apply)
- [ ] Code refactor (Modify the code structure, format the code, etc...)
- [x] Optimization. Including functional usability improvements and performance improvements.
- [ ] Dependency. Such as changes related to third-party components.
- [ ] Other.
## Checklist
_Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._
- [x] I have created an issue on (Fix #ISSUE) and described the bug/feature there in detail
- [x] Compiling and unit tests pass locally with my changes
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] If these changes need document changes, I have updated the document
- [ ] Any dependent changes have been merged
## Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...
Todo：
add some unit test of the rule
## Test
`````
#create table1
CREATE TABLE table1
(
siteid INT DEFAULT '10',
citycode SMALLINT,
username VARCHAR(32) DEFAULT '',
pv BIGINT SUM DEFAULT '0'
)
AGGREGATE KEY(siteid, citycode, username)
DISTRIBUTED BY HASH(siteid) BUCKETS 10
PROPERTIES("replication_num" = "1");
explain select * from table1 where 2=-2 OR citycode=0;
#before：PREDICATES: (FALSE OR `citycode` = 0)
#after：PREDICATES: `citycode` = 0
explain select * from table1 where (2=-2) OR (citycode=0 AND 1=1);
#before：PREDICATES: (FALSE OR (`citycode` = 0 AND TRUE))
#after：PREDICATES: `citycode` = 0
explain select * from table1 where (-2=2 or citycode=2) and (-2=2 or siteid=2);
#before PREDICATES: (FALSE OR `citycode` = 0) and (FALSE OR `siteid` = 2)
#after PREDICATES: `citycode` = 2, `siteid` = 2
`````
After add the rewrite 'OR' 'AND' rule, here are partial test cases which have passed :
Until now,the added rule passed all the test case and no err caused.
`````
# (true AND expr1) or (false AND expr2) ==> expr1
explain select * from table1 where (-5=-5 AND citycode=0) OR (3=2 AND siteid=2);
#PREDICATES: `citycode` = 0
explain select * from table1 where citycode=0 OR (false AND siteid=2);
#PREDICATES: `citycode` = 0
explain select * from table1 where citycode=0 OR (2=2 AND siteid=2);
# PREDICATES: (`citycode` = 0) OR (`siteid` = 2)
explain select * from table1 where citycode=0 OR ( siteid=2 AND 3!=3);
#PREDICATES: `citycode` = 0
# (false or expr1) and (expr2 or false) ==> expr1 and expr2
explain select * from table1 where (-2=2 OR citycode=0) and (siteid=2 OR 2=-2);
#PREDICATES: `citycode` = 0, `siteid` = 2
# (true or expr1) and (expr2 or false) ==> expr2
explain select * from table1 where (-5=-5 OR citycode=0) and (siteid=2 OR 2=-2);
#PREDICATES: `siteid` = 2
# (true or expr1) and (false or expr2) ==> expr2
explain select * from table1 where (-5=-5 OR citycode=0) and (3=2 OR siteid=2);
#PREDICATES: `siteid` = 2
# (true AND expr1) or (true AND expr2) ==> expr1 or expr2
explain select * from table1 where (-5=-5 AND citycode=0) OR (2=2 AND siteid=2);
#PREDICATES: (`citycode` = 0) OR (`siteid` = 2)
# false OR expr ==> expr
explain select * from table1 where 1-3=2 OR citycode=0;
#PREDICATES: `citycode` = 0
# (false OR true) OR expr ==> true
explain select * from table1 where (2+1=-2 OR 1=1) OR citycode=0;
#NULL PREDICATES
# false AND expr ==> false
explain select * from table1 where (2-3=-2) AND citycode=0;
#EMPTYSET
`````
--
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] zbtzbtzbt commented on a change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
zbtzbtzbt commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701015161
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,102 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * add Rewrite CompoundPredicates 'OR' 'AND' 'NOT' Rule
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ * Examples:
+ * OR:
Review comment:
ok,i will change it later~
--
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] morningman commented on a change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r702593054
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,89 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * Add rewrite CompoundPredicates 'OR' 'AND' rule
+ * 'OR' 'AND' rewrite rule
+ * case true and expr ==> expr
+ * case expr and true ==> expr
+ * case false or expr ==> expr
+ * case expr or false ==> expr
+ *
+ * case false or expr ==> false
+ * case expr or false ==> false
Review comment:
```suggestion
* case expr and false ==> false
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,89 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * Add rewrite CompoundPredicates 'OR' 'AND' rule
+ * 'OR' 'AND' rewrite rule
+ * case true and expr ==> expr
+ * case expr and true ==> expr
+ * case false or expr ==> expr
+ * case expr or false ==> expr
+ *
+ * case false or expr ==> false
Review comment:
```suggestion
* case false and expr ==> false
```
--
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] zbtzbtzbt commented on pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
zbtzbtzbt commented on pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#issuecomment-910255419
here is the final version.
--
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 #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#issuecomment-913357538
--
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 change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r700760493
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,102 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * add Rewrite CompoundPredicates 'OR' 'AND' 'NOT' Rule
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ * Examples:
+ * OR:
Review comment:
The comments here are duplicated with the following ones. I'd like to suggest removing this, just use the below comments.
```
/*
* 'OR' 'AND' rewrite rule
* case true AND expr ==> expr
* case expr AND true ==> expr
* case false Or expr ==> expr
* case expr Or false ==> expr
*
* case false AND expr ==> false
* case expr AND false ==> false
* case true Or expr ==> true
* case expr Or true ==> true
*
*/
```
--
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] zbtzbtzbt commented on a change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
zbtzbtzbt commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701547649
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,102 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * add Rewrite CompoundPredicates 'OR' 'AND' 'NOT' Rule
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ */
+
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+
+ /*
+ * 'OR' 'AND' rewrite rule
+ * case true AND expr ==> expr
Review comment:
ok，i will change those and commit，thanks for your code review，i will pay attention to these details staring today.
--
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 change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r696724744
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
Review comment:
`if (leftChild instanceof BoolLiteral) {`
Code style, add space before `(` and `{`. Please check all the code.
##########
File path: fe/fe-common/pom.xml
##########
@@ -54,6 +54,11 @@ under the License.
</profiles>
<dependencies>
+ <dependency>
+ <groupId>org.projectlombok</groupId>
Review comment:
Do you really need this?
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
Review comment:
Code style, space after `if`.
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralLeftChild.getValue()){
+ leftChild = new BoolLiteral(false);
+ result= rightChild;
+ }
+
+ }
+
+ //rightChild
Review comment:
`// rightChild`
Add space after `//`.
--
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 change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701071762
##########
File path: fe/pom.xml
##########
@@ -661,7 +661,7 @@ under the License.
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
- <version>1.18.16</version>
+ <version>1.18.20</version>
Review comment:
What do you mean by 'test null'?
--
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] zbtzbtzbt commented on a change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
zbtzbtzbt commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701017847
##########
File path: fe/pom.xml
##########
@@ -661,7 +661,7 @@ under the License.
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
- <version>1.18.16</version>
+ <version>1.18.20</version>
Review comment:
to support [test null](https://github.com/apache/incubator-doris/blob/79fd117d60bd053d892f8aaa8c9a4554a4b35ed7/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java#L1718),or build will fail
--
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 change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701540731
##########
File path: fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
##########
@@ -407,6 +407,16 @@ public static void beforeClass() throws Exception {
createView("create view test.tbl_null_column_view AS SELECT *,NULL as add_column FROM test.test1;");
+ createTable("CREATE TABLE test.table_or(" +
Review comment:
There is no need to create a new table for every test. We could reuse the existing table in the test class, e.g, `test.test1`.
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,102 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * add Rewrite CompoundPredicates 'OR' 'AND' 'NOT' Rule
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ */
+
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+
+ /*
+ * 'OR' 'AND' rewrite rule
+ * case true AND expr ==> expr
Review comment:
Typo.
It's better to unify the style, a.k.a. use uppercase or lowercase for all the keywords `and` and `or`, rather than uppercase `AND` and camelcase `Or`. Maybe just use lowercase `and` and `or` is enough.
--
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 change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701540731
##########
File path: fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java
##########
@@ -407,6 +407,16 @@ public static void beforeClass() throws Exception {
createView("create view test.tbl_null_column_view AS SELECT *,NULL as add_column FROM test.test1;");
+ createTable("CREATE TABLE test.table_or(" +
Review comment:
There is no need to create a new table for every test. We could reuse the existing table in the test class, e.g., `test.test1`.
--
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 change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r700742327
##########
File path: fe/pom.xml
##########
@@ -661,7 +661,7 @@ under the License.
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
- <version>1.18.16</version>
+ <version>1.18.20</version>
Review comment:
Why bump this version?
--
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] zbtzbtzbt commented on a change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
zbtzbtzbt commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r701015161
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,102 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+
+import java.util.List;
+
+/**
+ * add Rewrite CompoundPredicates 'OR' 'AND' 'NOT' Rule
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ * Examples:
+ * OR:
Review comment:
ok
--
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] morningman merged pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518
--
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] morningman commented on a change in pull request #6518: [Enhance] Add a rewrite rule of compoundPredicate 'OR' 'AND' to hit prefix index
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #6518:
URL: https://github.com/apache/incubator-doris/pull/6518#discussion_r696688746
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
Review comment:
```suggestion
// false OR expr ==> expr
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
Review comment:
There is no need to specify this situation, because the rule is recursively applied to the predicate, so it must support the mix predicate
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralLeftChild.getValue()){
+ leftChild = new BoolLiteral(false);
Review comment:
```suggestion
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralLeftChild.getValue()){
+ leftChild = new BoolLiteral(false);
+ result= rightChild;
+ }
+
+ }
+
+ //rightChild
+ // expr OR true ==> true
+ // expr OR false ==> expr
+ if(rightChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralRightChild = (BoolLiteral) rightChild;
+
+ if(boolLiteralRightChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralRightChild.getValue()){
Review comment:
```suggestion
} else {
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralLeftChild.getValue()){
+ leftChild = new BoolLiteral(false);
+ result= rightChild;
+ }
+
+ }
+
+ //rightChild
+ // expr OR true ==> true
+ // expr OR false ==> expr
+ if(rightChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralRightChild = (BoolLiteral) rightChild;
+
+ if(boolLiteralRightChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralRightChild.getValue()){
+ result= leftChild;
+ }
+ }
+
+ return result;
+ }
+
+ //rewrite AND
+ if(cp.getOp() == CompoundPredicate.Operator.AND) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true AND expr ==> expr
+ // false AND expr ==> false
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ leftChild = new BoolLiteral(true);
Review comment:
```suggestion
```
No need to assign again
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralLeftChild.getValue()){
+ leftChild = new BoolLiteral(false);
+ result= rightChild;
+ }
+
+ }
+
+ //rightChild
+ // expr OR true ==> true
+ // expr OR false ==> expr
+ if(rightChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralRightChild = (BoolLiteral) rightChild;
+
+ if(boolLiteralRightChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralRightChild.getValue()){
Review comment:
same as follow AND cases
##########
File path: fe/fe-core/src/main/java/org/apache/doris/rewrite/CompoundPredicateWriteRule.java
##########
@@ -0,0 +1,134 @@
+// 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.rewrite;
+
+import org.apache.doris.analysis.Analyzer;
+import org.apache.doris.analysis.BoolLiteral;
+import org.apache.doris.analysis.CompoundPredicate;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.common.AnalysisException;
+import java.util.List;
+
+/**
+ * Rewrites CompoundPredicates 'OR' 'AND' 'NOT'
+ * CompoundPredicate.
+ * It can be applied to pre-analysis expr trees and therefore does not reanalyze
+ * the transformation output itself.
+ *
+ * Examples:
+ * OR:
+ * (-2==2 OR city_id=2) ==> city_id=2
+ * (city_id=2 OR -2==2) ==> city_id=2
+ * -5!=-5 OR citycode=0 ==> citycode=0
+ * AND:
+ * (citycode=0 AND 1=1) ==> citycode=0
+ * -5=-5 AND citycode=0 AND 2=2 ==> citycode=0
+ *
+ * Support complex 'AND' 'OR' mixed use
+ * (-2=2 or citycode=2) and (-2=2 or siteid=2) ==> citycode=2 and siteid=2
+ * (-5=-5 AND citycode=0) OR (3=2 AND siteid=2) ==> citycode=0
+ * more test case,please refer to doris#issue6499
+ */
+public class CompoundPredicateWriteRule implements ExprRewriteRule {
+ public static ExprRewriteRule INSTANCE = new CompoundPredicateWriteRule();
+ @Override
+ public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
+
+ if (!(expr instanceof CompoundPredicate)) return expr;
+ CompoundPredicate cp = (CompoundPredicate) expr;
+ Expr result = expr;
+
+ List<Expr> children = cp.getChildren();
+ if (children.size() != 2) {
+ return expr;
+ }
+ //rewrite OR
+ if(cp.getOp() == CompoundPredicate.Operator.OR) {
+
+ Expr leftChild=cp.getChild(0);
+ Expr rightChild=cp.getChild(1);
+
+ //leftChild
+ // true OR expr ==> true
+ // false AND expr ==> expr
+ if(leftChild instanceof BoolLiteral){
+ BoolLiteral boolLiteralLeftChild = (BoolLiteral) leftChild;
+
+ if(boolLiteralLeftChild.getValue()){
+ return new BoolLiteral(true);
+ }else if(!boolLiteralLeftChild.getValue()){
Review comment:
```suggestion
} else {
```
--
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