morrySnow commented on code in PR #36111:
URL: https://github.com/apache/doris/pull/36111#discussion_r1636136314
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/Nondeterministic.java:
##########
@@ -24,4 +24,10 @@
*
*/
public interface Nondeterministic extends ExpressionTrait {
+
+ /**
+ * Identify the function is deterministic or not, such as UnixTimestamp,
when it's children is not empty
+ * it's deterministic
+ */
+ boolean isDeterministic();
Review Comment:
add this interface to expression with default implement check all check
isDeterministic. Nondeterministic default return false. unixTimestamp do not
implement Nondeterministic anymore. all check `isntance of Nondeterministic`
change to check `expression.isDeterministic()`.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/NondeterministicFunctionCollector.java:
##########
@@ -17,31 +17,66 @@
package org.apache.doris.nereids.trees.plans.visitor;
-import org.apache.doris.nereids.trees.TreeNode;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.functions.Nondeterministic;
import org.apache.doris.nereids.trees.plans.Plan;
+import
org.apache.doris.nereids.trees.plans.visitor.NondeterministicFunctionCollector.FunctionCollectContext;
+import java.util.Collection;
+import java.util.LinkedList;
import java.util.List;
+import java.util.Set;
/**
* Collect the nondeterministic expr in plan, these expressions will be put
into context
*/
public class NondeterministicFunctionCollector
- extends DefaultPlanVisitor<Void, List<TreeNode<Expression>>> {
+ extends DefaultPlanVisitor<Void, FunctionCollectContext> {
- public static final NondeterministicFunctionCollector INSTANCE
- = new NondeterministicFunctionCollector();
+ public static final NondeterministicFunctionCollector INSTANCE = new
NondeterministicFunctionCollector();
@Override
- public Void visit(Plan plan, List<TreeNode<Expression>>
collectedExpressions) {
+ public Void visit(Plan plan, FunctionCollectContext collectContext) {
List<? extends Expression> expressions = plan.getExpressions();
if (expressions.isEmpty()) {
- return super.visit(plan, collectedExpressions);
+ return super.visit(plan, collectContext);
+ }
+ for (Expression expression : expressions) {
+ Set<Expression> nondeterministicFunctions =
expression.collect(expr ->
+ expr instanceof Nondeterministic && !((Nondeterministic)
expr).isDeterministic());
+ collectContext.addAllExpressions(nondeterministicFunctions);
+ }
+ return super.visit(plan, collectContext);
+ }
+
+ /**
+ * Function collect context, collected expressions would be added to
collectedExpressions,
+ * the expression in whiteFunctionSet would not be collected
+ */
+ public static class FunctionCollectContext {
+ private final List<Expression> collectedExpressions = new
LinkedList<>();
+
+ private FunctionCollectContext() {
+ }
+
+ public static FunctionCollectContext of() {
+ return new FunctionCollectContext();
+ }
+
+ public void addExpression(Expression expression) {
+ if (expression != null) {
+ this.collectedExpressions.add(expression);
+ }
+ }
+
+ public void addAllExpressions(Collection<? extends Expression>
expressions) {
+ if (expressions != null) {
+ this.collectedExpressions.addAll(expressions);
+ }
+ }
+
+ public List<Expression> getCollectedExpressions() {
+ return collectedExpressions;
}
- expressions.forEach(expression -> {
-
collectedExpressions.addAll(expression.collect(Nondeterministic.class::isInstance));
Review Comment:
i think u could collected isDeterministic == true after move this interface
into expression
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/visitor/NondeterministicFunctionCollector.java:
##########
@@ -17,31 +17,66 @@
package org.apache.doris.nereids.trees.plans.visitor;
-import org.apache.doris.nereids.trees.TreeNode;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.functions.Nondeterministic;
import org.apache.doris.nereids.trees.plans.Plan;
+import
org.apache.doris.nereids.trees.plans.visitor.NondeterministicFunctionCollector.FunctionCollectContext;
+import java.util.Collection;
+import java.util.LinkedList;
import java.util.List;
+import java.util.Set;
/**
* Collect the nondeterministic expr in plan, these expressions will be put
into context
*/
public class NondeterministicFunctionCollector
- extends DefaultPlanVisitor<Void, List<TreeNode<Expression>>> {
+ extends DefaultPlanVisitor<Void, FunctionCollectContext> {
- public static final NondeterministicFunctionCollector INSTANCE
- = new NondeterministicFunctionCollector();
+ public static final NondeterministicFunctionCollector INSTANCE = new
NondeterministicFunctionCollector();
@Override
- public Void visit(Plan plan, List<TreeNode<Expression>>
collectedExpressions) {
+ public Void visit(Plan plan, FunctionCollectContext collectContext) {
List<? extends Expression> expressions = plan.getExpressions();
if (expressions.isEmpty()) {
- return super.visit(plan, collectedExpressions);
+ return super.visit(plan, collectContext);
+ }
+ for (Expression expression : expressions) {
+ Set<Expression> nondeterministicFunctions =
expression.collect(expr ->
+ expr instanceof Nondeterministic && !((Nondeterministic)
expr).isDeterministic());
+ collectContext.addAllExpressions(nondeterministicFunctions);
+ }
+ return super.visit(plan, collectContext);
+ }
+
+ /**
+ * Function collect context, collected expressions would be added to
collectedExpressions,
+ * the expression in whiteFunctionSet would not be collected
+ */
+ public static class FunctionCollectContext {
+ private final List<Expression> collectedExpressions = new
LinkedList<>();
Review Comment:
do not use java's linkedList in any case. it has poor perf with no adv with
ArrayList
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
