morrySnow commented on code in PR #44748:
URL: https://github.com/apache/doris/pull/44748#discussion_r1862913381
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/LogicalProperties.java:
##########
@@ -100,6 +94,10 @@ public LogicalProperties(Supplier<List<Slot>>
outputSupplier,
);
}
+ public LogicalProperties withOutputSupplier(Supplier<List<Slot>>
outputSupplier) {
Review Comment:
not used anymore
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/EncodeStr.java:
##########
@@ -17,8 +17,17 @@
package org.apache.doris.nereids.trees.expressions.functions.scalar;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
+
/**
* Encode_as_XXXInt
*/
-public interface EncodeStrToInteger {
+public abstract class EncodeStr extends ScalarFunction implements
UnaryExpression {
Review Comment:
nit: w'd not use abbreviation. Str -> String
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/Plan.java:
##########
@@ -232,4 +233,8 @@ default String getGroupIdAsString() {
default String getGroupIdWithPrefix() {
return "@" + getGroupIdAsString();
}
+
+ default Plan replaceExpressions(Map<? extends Expression, ? extends
Expression> replaceMap) {
Review Comment:
this default impl is dangerous, please impl for all node types. Or Impl it
as throw exception
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java:
##########
@@ -217,7 +217,7 @@ public OlapTable getTable() {
@Override
public String toString() {
return Utils.toSqlString("LogicalOlapScan",
- "qualified", qualifiedName(),
Review Comment:
why remove quailied name? it is useful if two table name is same but in
different db
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalProject.java:
##########
@@ -278,4 +278,22 @@ public void computeFd(DataTrait.Builder builder) {
}
}
}
+
+ @Override
+ public Plan replaceExpressions(Map<? extends Expression, ? extends
Expression> replaceMap) {
+ List<NamedExpression> newProjections = new ArrayList<>();
+ boolean changed = false;
+ for (NamedExpression expr : getProjects()) {
+ if (replaceMap.containsKey(expr) && replaceMap.get(expr)
instanceof NamedExpression) {
Review Comment:
only replace top level expression? the impl is not match the interface's
name. should do tree traverse replacement or rename this interface as
replaceRootExpression. if replacing only root is expected, we should not add
this interface in Plan, because it is not very general
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownEncodeSlot.java:
##########
@@ -0,0 +1,180 @@
+// 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.rewrite;
+
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.EncodeStr;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.nereids.util.PlanUtils;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * push down encode_as_int(slot) down
+ * example:
+ * group by x
+ * -->project(encode_as_int(A) as x)
+ * -->Any(A)
+ * -->project(A)
+ * --> scan
+ * =>
+ * group by x
+ * -->project(x)
+ * -->Any(x)
+ * --> project(encode_as_int(A) as x)
+ * -->scan
+ * Note:
+ * do not push down encode if encode.child() is not slot,
+ * example
+ * group by encode_as_int(A + B)
+ * --> any(A, B)
+ */
+public class PushDownEncodeSlot extends OneRewriteRuleFactory {
Review Comment:
maybe we could let this rule more general. add a interface for expression
that could be push down to speed up exectuion. and then put them all down
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownEncodeSlot.java:
##########
@@ -0,0 +1,180 @@
+// 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.rewrite;
+
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.EncodeStr;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.nereids.util.PlanUtils;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * push down encode_as_int(slot) down
+ * example:
+ * group by x
+ * -->project(encode_as_int(A) as x)
+ * -->Any(A)
+ * -->project(A)
+ * --> scan
+ * =>
+ * group by x
+ * -->project(x)
+ * -->Any(x)
+ * --> project(encode_as_int(A) as x)
+ * -->scan
+ * Note:
+ * do not push down encode if encode.child() is not slot,
+ * example
+ * group by encode_as_int(A + B)
+ * --> any(A, B)
+ */
+public class PushDownEncodeSlot extends OneRewriteRuleFactory {
+
+ @Override
+ public Rule build() {
+ return logicalProject()
+ .when(this::containsEncode)
+ .when(project -> !(project.child() instanceof
LogicalCatalogRelation))
+ .then(project -> pushDownEncodeSlot(project))
Review Comment:
```suggestion
.then(this::pushDownEncodeSlot)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
