924060929 commented on code in PR #11589:
URL: https://github.com/apache/doris/pull/11589#discussion_r946338558
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/MergeConsecutiveProjects.java:
##########
@@ -50,6 +52,40 @@
*/
public class MergeConsecutiveProjects extends OneRewriteRuleFactory {
+
+ private static class ExpressionReplacer extends
DefaultExpressionRewriter<Map<Expression, Expression>> {
+ public static final ExpressionReplacer INSTANCE = new
ExpressionReplacer();
+
+ /**
+ * case 1: project(project(alias, alias) -> alias, alias) =>
project(alias, alias)
+ * case 2: project(project(alias, alias) -> slot ref, slot ref) =>
project(project(alias, alias))
+ * case 3: others: use ExpressionReplacer.
Review Comment:
this comment should add some concrete example.
like this
```java
case 1:
project(alias(c) + 1, alias(c) + 1)
| =>
project(slotRef(a) + 1 + 1, slotRef(b) + 1 + 1)
project(slotRef(a) + 1 as c, slotRef(b) + 1 as d)
```
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/ViewTest.java:
##########
@@ -0,0 +1,161 @@
+// 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.trees.expressions;
+
+import org.apache.doris.nereids.NereidsPlanner;
+import org.apache.doris.nereids.StatementContext;
+import org.apache.doris.nereids.glue.translator.PhysicalPlanTranslator;
+import org.apache.doris.nereids.glue.translator.PlanTranslatorContext;
+import org.apache.doris.nereids.parser.NereidsParser;
+import org.apache.doris.nereids.properties.PhysicalProperties;
+import org.apache.doris.nereids.rules.analysis.EliminateAliasNode;
+import org.apache.doris.nereids.rules.rewrite.logical.MergeConsecutiveProjects;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalPlan;
+import org.apache.doris.nereids.util.MemoTestUtils;
+import org.apache.doris.nereids.util.PatternMatchSupported;
+import org.apache.doris.nereids.util.PlanChecker;
+import org.apache.doris.utframe.TestWithFeService;
+
+import com.google.common.collect.Lists;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+public class ViewTest extends TestWithFeService implements
PatternMatchSupported {
+ private final List<String> testSql = Lists.newArrayList(
+ "SELECT * FROM V1",
+ "SELECT * FROM V2",
+ "SELECT * FROM V3",
+ "SELECT * FROM T1 JOIN (SELECT * FROM V1) T ON T1.ID1 = T.ID1",
+ "SELECT * FROM T2 JOIN (SELECT * FROM V2) T ON T2.ID2 = T.ID2",
+ "SELECT Y.ID2 FROM (SELECT * FROM V3) Y",
+ "SELECT * FROM (SELECT * FROM V1 JOIN V2 ON V1.ID1 = V2.ID2) X
JOIN (SELECT * FROM V1 JOIN V3 ON V1.ID1 = V3.ID2) Y ON X.ID1 = Y.ID3"
+ );
+
+ @Override
+ protected void runBeforeAll() throws Exception {
+ createDatabase("test");
+ connectContext.setDatabase("default_cluster:test");
+ createTables(
+ "CREATE TABLE IF NOT EXISTS T1 (\n"
+ + " ID1 bigint,\n"
+ + " SCORE1 bigint\n"
+ + ")\n"
+ + "DUPLICATE KEY(ID1)\n"
+ + "DISTRIBUTED BY HASH(ID1) BUCKETS 1\n"
+ + "PROPERTIES (\n"
+ + " \"replication_num\" = \"1\"\n"
+ + ")\n",
+ "CREATE TABLE IF NOT EXISTS T2 (\n"
+ + " ID2 bigint,\n"
+ + " SCORE2 bigint\n"
+ + ")\n"
+ + "DUPLICATE KEY(ID2)\n"
+ + "DISTRIBUTED BY HASH(ID2) BUCKETS 1\n"
+ + "PROPERTIES (\n"
+ + " \"replication_num\" = \"1\"\n"
+ + ")\n",
+ "CREATE TABLE IF NOT EXISTS T3 (\n"
+ + " ID3 bigint,\n"
+ + " SCORE3 bigint\n"
+ + ")\n"
+ + "DUPLICATE KEY(ID3)\n"
+ + "DISTRIBUTED BY HASH(ID3) BUCKETS 1\n"
+ + "PROPERTIES (\n"
+ + " \"replication_num\" = \"1\"\n"
+ + ")\n"
+ );
+ createView("CREATE VIEW V1 AS SELECT * FROM T1");
+ createView("CREATE VIEW V2 AS SELECT * FROM T2");
+ createView("CREATE VIEW V3 AS SELECT * FROM T3 JOIN (SELECT * FROM V2)
T ON T3.ID3 = T.ID2");
+ }
+
+ @Override
+ protected void runBeforeEach() throws Exception {
+ NamedExpressionUtil.clear();
+ }
+
+ @Test
+ public void testTranslateAllCase() throws Exception {
+ // check whether they can be translated.
+ for (String sql : testSql) {
+ NamedExpressionUtil.clear();
+ System.out.println("\n\n***** " + sql + " *****\n\n");
+ StatementContext statementContext =
MemoTestUtils.createStatementContext(connectContext, sql);
+ PhysicalPlan plan = new NereidsPlanner(statementContext).plan(
+ new NereidsParser().parseSingle(sql),
+ PhysicalProperties.ANY
+ );
+ // Just to check whether translate will throw exception
+ new PhysicalPlanTranslator().translatePlan(plan, new
PlanTranslatorContext());
+ }
+ }
+
+ @Test
+ public void testSimpleViewMergeProjects() {
+ // FieldChecker projectCheck = new
FieldChecker(ImmutableList.of("projects"));
+ PlanChecker.from(connectContext)
+ .analyze(testSql.get(0))
Review Comment:
nit: copy the sql string to here
the reason is
1. don't reuse variables which across many lines, because of not conducive
to reading and comparison
2. there are not many associations, unless it depends on more than one
variable, such as：input and output pair
##########
regression-test/suites/nereids_syntax_p0/view.groovy:
##########
@@ -0,0 +1,99 @@
+// 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.
+
+suite("view") {
+ sql """
+ SET enable_vectorized_engine=true
+ """
+
+ sql """
+ SET enable_nereids_planner=true
+ """
+
+ sql """
+ create view if not exists v1 as
+ select *
+ from customer
+ """
+
+ sql """
+ create view if not exists v2 as
+ select *
+ from lineorder
+ """
+
+ sql """
+ create view if not exists v3 as
+ select *
+ from v1 join (
+ select *
+ from v2
+ ) t
+ on v1.c_custkey = t.lo_custkey;
+ """
+
+ qt_select_1 """
+ select *
+ from v1
+ order by v1.c_custkey
+ """
+
+ qt_select_2 """
+ select *
+ from v2
+ order by v2.lo_custkey
+ """
+
+ qt_select_3 """
+ select *
+ from v3
+ order by v3.c_custkey, v3.lo_orderkey
+ """
+
+ qt_select_4 """
+ select *
+ from customer c join (
+ select *
+ from v1
+ ) t
+ on c.c_custkey = t.c_custkey
+ order by c.c_custkey, t.c_custkey
+ """
+
+ qt_select_5 """
+ select *
+ from lineorder l join (
+ select *
+ from v2
+ ) t
+ on l.lo_custkey = t.lo_custkey
+ order by l.lo_custkey, t.lo_custkey
+ """
+
+ qt_select_6 """
+ select * from (
+ select *
+ from part p
+ join v2 on p.p_partkey = v2.lo_partkey) t1
+ join (
+ select *
+ from supplier s
+ join v3 on s.s_region = v3.c_region) t2
+ on t1.p_partkey = t2.lo_partkey
+ order by t1.lo_custkey, t1.p_partkey, t2.s_suppkey, t2.c_custkey,
t2.lo_orderkey
+ """
+}
Review Comment:
add a newline here
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java:
##########
@@ -141,10 +146,15 @@ private void analyze() {
cascadesContext.newAnalyzer().analyze();
}
+ private void finalizeAnalyze() {
+ new FinalizeAnalyzeJob(cascadesContext).execute();
+ }
Review Comment:
ditto
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java:
##########
@@ -108,6 +110,9 @@ public PhysicalPlan plan(LogicalPlan plan,
PhysicalProperties outputProperties)
// resolve column, table and function
analyze();
+ // eliminate sub-query and alias node
+ finalizeAnalyze();
Review Comment:
remove this line, the finalizeAnalyze function already exists in
NereidsPlanner#analyze and more reasonable, because finalizeAnalyze is a part
of analyze stage
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/ViewTest.java:
##########
@@ -0,0 +1,161 @@
+// 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.trees.expressions;
+
+import org.apache.doris.nereids.NereidsPlanner;
+import org.apache.doris.nereids.StatementContext;
+import org.apache.doris.nereids.glue.translator.PhysicalPlanTranslator;
+import org.apache.doris.nereids.glue.translator.PlanTranslatorContext;
+import org.apache.doris.nereids.parser.NereidsParser;
+import org.apache.doris.nereids.properties.PhysicalProperties;
+import org.apache.doris.nereids.rules.analysis.EliminateAliasNode;
+import org.apache.doris.nereids.rules.rewrite.logical.MergeConsecutiveProjects;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalPlan;
+import org.apache.doris.nereids.util.MemoTestUtils;
+import org.apache.doris.nereids.util.PatternMatchSupported;
+import org.apache.doris.nereids.util.PlanChecker;
+import org.apache.doris.utframe.TestWithFeService;
+
+import com.google.common.collect.Lists;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+public class ViewTest extends TestWithFeService implements
PatternMatchSupported {
+ private final List<String> testSql = Lists.newArrayList(
+ "SELECT * FROM V1",
+ "SELECT * FROM V2",
+ "SELECT * FROM V3",
+ "SELECT * FROM T1 JOIN (SELECT * FROM V1) T ON T1.ID1 = T.ID1",
+ "SELECT * FROM T2 JOIN (SELECT * FROM V2) T ON T2.ID2 = T.ID2",
+ "SELECT Y.ID2 FROM (SELECT * FROM V3) Y",
+ "SELECT * FROM (SELECT * FROM V1 JOIN V2 ON V1.ID1 = V2.ID2) X
JOIN (SELECT * FROM V1 JOIN V3 ON V1.ID1 = V3.ID2) Y ON X.ID1 = Y.ID3"
+ );
+
+ @Override
+ protected void runBeforeAll() throws Exception {
+ createDatabase("test");
+ connectContext.setDatabase("default_cluster:test");
+ createTables(
+ "CREATE TABLE IF NOT EXISTS T1 (\n"
+ + " ID1 bigint,\n"
+ + " SCORE1 bigint\n"
+ + ")\n"
+ + "DUPLICATE KEY(ID1)\n"
+ + "DISTRIBUTED BY HASH(ID1) BUCKETS 1\n"
+ + "PROPERTIES (\n"
+ + " \"replication_num\" = \"1\"\n"
+ + ")\n",
+ "CREATE TABLE IF NOT EXISTS T2 (\n"
+ + " ID2 bigint,\n"
+ + " SCORE2 bigint\n"
+ + ")\n"
+ + "DUPLICATE KEY(ID2)\n"
+ + "DISTRIBUTED BY HASH(ID2) BUCKETS 1\n"
+ + "PROPERTIES (\n"
+ + " \"replication_num\" = \"1\"\n"
+ + ")\n",
+ "CREATE TABLE IF NOT EXISTS T3 (\n"
+ + " ID3 bigint,\n"
+ + " SCORE3 bigint\n"
+ + ")\n"
+ + "DUPLICATE KEY(ID3)\n"
+ + "DISTRIBUTED BY HASH(ID3) BUCKETS 1\n"
+ + "PROPERTIES (\n"
+ + " \"replication_num\" = \"1\"\n"
+ + ")\n"
+ );
+ createView("CREATE VIEW V1 AS SELECT * FROM T1");
+ createView("CREATE VIEW V2 AS SELECT * FROM T2");
+ createView("CREATE VIEW V3 AS SELECT * FROM T3 JOIN (SELECT * FROM V2)
T ON T3.ID3 = T.ID2");
+ }
+
+ @Override
+ protected void runBeforeEach() throws Exception {
+ NamedExpressionUtil.clear();
+ }
+
+ @Test
+ public void testTranslateAllCase() throws Exception {
+ // check whether they can be translated.
+ for (String sql : testSql) {
+ NamedExpressionUtil.clear();
+ System.out.println("\n\n***** " + sql + " *****\n\n");
+ StatementContext statementContext =
MemoTestUtils.createStatementContext(connectContext, sql);
+ PhysicalPlan plan = new NereidsPlanner(statementContext).plan(
+ new NereidsParser().parseSingle(sql),
+ PhysicalProperties.ANY
+ );
+ // Just to check whether translate will throw exception
+ new PhysicalPlanTranslator().translatePlan(plan, new
PlanTranslatorContext());
+ }
+ }
+
+ @Test
+ public void testSimpleViewMergeProjects() {
+ // FieldChecker projectCheck = new
FieldChecker(ImmutableList.of("projects"));
+ PlanChecker.from(connectContext)
+ .analyze(testSql.get(0))
+ .applyTopDown(new EliminateAliasNode())
+ .applyTopDown(new MergeConsecutiveProjects())
+ .matches(
+ logicalProject(
+ logicalOlapScan()
+ )
+ );
+ }
+
+ @Test
+ public void testNestedView() {
+ PlanChecker.from(connectContext)
+ .analyze(testSql.get(6))
Review Comment:
ditto
--
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
