From 423a577a6dc029cd129f45ec4e12918a41f7a8f6 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 15 Aug 2024 07:04:17 +0000 Subject: [PATCH] [Enhancement] shorten FE plan time in some corner case (backport #49137) (#49222) Co-authored-by: before-Sunrise <71162020+before-Sunrise@users.noreply.github.com> --- .../com/starrocks/sql/optimizer/Utils.java | 23 ++++-------------- .../scalar/CompoundPredicateOperator.java | 4 ++-- .../scalar/ExtractCommonPredicateRule.java | 2 +- .../scalar/NormalizePredicateRule.java | 21 ++++++++++++---- .../scalar/NormalizePredicateRuleTest.java | 24 +++++++++++++++++++ 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java index 9a4fc808cf870..d9a734e4c2c10 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java @@ -149,29 +149,15 @@ private static void extractDisjunctiveImpl(ScalarOperator root, List extractColumnRef(ScalarOperator root) { - if (null == root || !root.isVariable()) { + if (null == root) { return new LinkedList<>(); } - LinkedList list = new LinkedList<>(); - if (OperatorType.VARIABLE.equals(root.getOpType())) { - list.add((ColumnRefOperator) root); - return list; - } - - for (ScalarOperator child : root.getChildren()) { - list.addAll(extractColumnRef(child)); - } - - return list; + return root.getColumnRefs(); } public static int countColumnRef(ScalarOperator root) { - return countColumnRef(root, 0); - } - - private static int countColumnRef(ScalarOperator root, int count) { - if (null == root || !root.isVariable()) { + if (null == root) { return 0; } @@ -179,8 +165,9 @@ private static int countColumnRef(ScalarOperator root, int count) { return 1; } + int count = 0; for (ScalarOperator child : root.getChildren()) { - count += countColumnRef(child, count); + count += countColumnRef(child); } return count; diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CompoundPredicateOperator.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CompoundPredicateOperator.java index 7e43ad32a0215..7630ed2031760 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CompoundPredicateOperator.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/scalar/CompoundPredicateOperator.java @@ -38,8 +38,8 @@ public class CompoundPredicateOperator extends PredicateOperator { // AND AND // / \ / \ // subT1 a+1 And subT5 - // / \ - // subT3 a+1 + // / \ + // subT3 a+1 private int compoundTreeLeafNodeNumber; private Set compoundTreeUniqueLeaves; diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/ExtractCommonPredicateRule.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/ExtractCommonPredicateRule.java index 5bfeb5067deb1..97de3712a5f4e 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/ExtractCommonPredicateRule.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/ExtractCommonPredicateRule.java @@ -50,7 +50,7 @@ public ScalarOperator visitCompoundPredicate(CompoundPredicateOperator predicate List> orAndPredicates = Lists.newArrayList(); for (ScalarOperator or : orLists) { - orAndPredicates.add(Lists.newArrayList(Utils.extractConjuncts(or))); + orAndPredicates.add(Utils.extractConjuncts(or)); } // extract common predicate diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRule.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRule.java index 2b51e1e3be90d..047e2cb028cc9 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRule.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRule.java @@ -145,9 +145,9 @@ public ScalarOperator visitCompoundPredicate(CompoundPredicateOperator predicate @Nullable private Optional getOptimizedCompoundTree(CompoundPredicateOperator parent) { // reset node first So we can apply NormalizePredicateRule to one tree many times - parent.setCompoundTreeUniqueLeaves(Sets.newLinkedHashSet()); + parent.setCompoundTreeUniqueLeaves(null); parent.setCompoundTreeLeafNodeNumber(0); - Set compoundTreeUniqueLeaves = parent.getCompoundTreeUniqueLeaves(); + Set compoundTreeUniqueLeaves = null; for (ScalarOperator child : parent.getChildren()) { if (child != null) { @@ -157,12 +157,19 @@ private Optional getOptimizedCompoundTree(CompoundPredicateOpera (parent.isOr() && OperatorType.COMPOUND.equals(child.getOpType()) && ((CompoundPredicateOperator) child).isOr())) { CompoundPredicateOperator compoundChild = (CompoundPredicateOperator) (child); - compoundTreeUniqueLeaves.addAll(compoundChild.getCompoundTreeUniqueLeaves()); + if (compoundTreeUniqueLeaves == null) { + compoundTreeUniqueLeaves = compoundChild.getCompoundTreeUniqueLeaves(); + } else { + compoundTreeUniqueLeaves.addAll(compoundChild.getCompoundTreeUniqueLeaves()); + } parent.setCompoundTreeLeafNodeNumber( compoundChild.getCompoundTreeLeafNodeNumber() + parent.getCompoundTreeLeafNodeNumber()); } else { // child is leaf node in compound tree // we cache CompoundPredicate's hash value to eliminate duplicate calculations + if (compoundTreeUniqueLeaves == null) { + compoundTreeUniqueLeaves = Sets.newLinkedHashSet(); + } compoundTreeUniqueLeaves.add(new HashCachedScalarOperator(child)); parent.setCompoundTreeLeafNodeNumber(1 + parent.getCompoundTreeLeafNodeNumber()); } @@ -176,6 +183,7 @@ private Optional getOptimizedCompoundTree(CompoundPredicateOpera } } } + parent.setCompoundTreeUniqueLeaves(compoundTreeUniqueLeaves); // this tree can be optimized if (compoundTreeUniqueLeaves.size() != parent.getCompoundTreeLeafNodeNumber()) { @@ -247,7 +255,12 @@ public ScalarOperator visitInPredicate(InPredicateOperator predicate, ScalarOper result.add(newOp); }); - return isIn ? Utils.compoundOr(result) : Utils.compoundAnd(result); + ScalarOperator res = isIn ? Utils.compoundOr(result) : Utils.compoundAnd(result); + if (res instanceof CompoundPredicateOperator) { + return visitCompoundPredicate((CompoundPredicateOperator) res, context); + } else { + return res; + } } // rewrite collection element to subfiled diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRuleTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRuleTest.java index 23b78a68c2e9b..d92662dba2088 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRuleTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/optimizer/rewrite/scalar/NormalizePredicateRuleTest.java @@ -118,4 +118,28 @@ public void testCompound() { assertTrue(result.isConstantTrue()); } + + @Test + public void testCompound1() { + NormalizePredicateRule rule = new NormalizePredicateRule(); + ScalarOperatorRewriteContext context = new ScalarOperatorRewriteContext(); + + InPredicateOperator inOp = new InPredicateOperator( + true, + ConstantOperator.createInt(1), + new ColumnRefOperator(0, Type.INT, "col1", true), + new ColumnRefOperator(0, Type.INT, "col1", true) + ); + + CompoundPredicateOperator compoundPredicateOperator = + new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.AND, inOp, + new BinaryPredicateOperator(BinaryType.GE, + ConstantOperator.createInt(1), + new ColumnRefOperator(1, Type.INT, "test1", true)) + ); + + ScalarOperatorRewriter operatorRewriter = new ScalarOperatorRewriter(); + ScalarOperator res = + operatorRewriter.rewrite(compoundPredicateOperator, Lists.newArrayList(new NormalizePredicateRule())); + } } \ No newline at end of file