Refactor ScanOperator to eliminate variability #375

HangyuanLiu · 2021-09-23T13:03:54Z

HangyuanLiu · 2021-09-24T12:54:15Z

run starrocks_fe_unittest

kangkaisen · 2021-09-26T03:00:33Z

...e/src/main/java/com/starrocks/sql/optimizer/rule/transformation/PruneHiveScanColumnRule.java

+                        .collect(Collectors.toSet());
+        scanColumns.addAll(Utils.extractColumnRef(scanOperator.getPredicate()));
+
+        if (scanColumns.size() == 0 || scanOperator.getPartitionColumns().containsAll(scanColumns)) {


Need comment

kangkaisen · 2021-09-26T03:00:49Z

...e/src/main/java/com/starrocks/sql/optimizer/rule/transformation/PruneHiveScanColumnRule.java

+            int smallestIndex = -1;
+            int smallestColumnLength = Integer.MAX_VALUE;
+            for (int index = 0; index < outputColumns.size(); ++index) {
+                if (scanOperator.getPartitionColumns().contains(outputColumns.get(index).getName())) {


Need comment

kangkaisen · 2021-09-26T03:02:04Z

fe/fe-core/src/test/java/com/starrocks/sql/optimizer/MVRewriteTest.java

+        starRocksAssert.withMaterializedView(createEmpsMVSQL);
+        //.query(query).explainContains(QUERY_USE_EMPS_MV, 2);
+
+        System.out.println("FIXME : " + starRocksAssert.query(query).explainQuery());


kangkaisen · 2021-09-26T03:02:21Z

fe/fe-core/src/test/java/com/starrocks/sql/optimizer/MVRewriteTest.java

@@ -488,7 +488,10 @@ public void testUnionAll() throws Exception {
                EMPS_TABLE_NAME + " order by empid, deptno;";
        String query = "select empid, deptno from " + EMPS_TABLE_NAME + " where empid >1 union all select empid,"
                + " deptno from " + EMPS_TABLE_NAME + " where empid <0;";
-        starRocksAssert.withMaterializedView(createEmpsMVSQL).query(query).explainContains(QUERY_USE_EMPS_MV, 2);
+        starRocksAssert.withMaterializedView(createEmpsMVSQL);


The refactor shouldn't change the UT result

kangkaisen · 2021-09-26T03:03:00Z

fe/fe-core/src/test/java/com/starrocks/sql/plan/PlanFragmentWithCostTest.java

@@ -465,9 +467,10 @@ public void testRollUp() throws Exception {
    public void testMV() throws Exception {
        String sql = "select count(distinct k7), count(distinct k8) from duplicate_table_with_null;";
        String planFragment = getFragmentPlan(sql);
+        System.out.println("FIXME : " + planFragment);


kangkaisen · 2021-09-26T03:04:26Z

fe/fe-core/src/test/java/com/starrocks/sql/optimizer/MVRewriteTest.java

@@ -980,4 +984,13 @@ public void testJoinProjectRewrite() throws Exception {
        query = "select approx_count_distinct(salary) from emps left outer join depts on emps.time = depts.time";
        starRocksAssert.query(query).explainContains("emps_mv");
    }
+
+    @Test
+    public void test1() throws Exception {


Need a concrete name

kangkaisen · 2021-09-26T03:08:35Z

...-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/PruneScanColumnRule.java

+                        ((LogicalOlapScanOperator) scanOperator).getDistributionSpec(),
+                        scanOperator.getLimit(),
+                        scanOperator.getPredicate());
+                newScanOperator.setSelectedIndexId(olapScanOperator.getSelectedIndexId());


Why still call set method here?

kangkaisen · 2021-09-26T03:10:11Z

...-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/PruneScanColumnRule.java

-                .collect(Collectors.toSet());
-        scanOutputColumns.addAll(Utils.extractColumnRef(scanOperator.getPredicate()));
-        scanOperator.tryExtendOutputColumns(scanOutputColumns);
+        Set<ColumnRefOperator> outputColumns =


should keep the comment

Seaven · 2021-09-26T03:18:50Z

fe/fe-core/src/main/java/com/starrocks/sql/optimizer/transformer/RelationTransformer.java

+                    new LogicalEsScanOperator(node.getTable(), outputVariables, colRefToColumnMetaMap.build(),
+                            columnMetaToColRefMap, -1,
+                            null);


-1 can be replace to DEFAULT_NONE_LIMIT or NONE_LIMIT

Most operator don't need set limit/predicate, canbe support a construnct method don't give limit/predicate

By the way, I think support builder mode(like Statistics.Builder) will make it easier to use?

1、There are too many modified codes at present, exceeding the limit of a review. I can submit another PR to modify this question
2、Can be provided, such as the first build in relationTransformer, but I do not recommend using this constructor

Youngwb · 2021-09-26T13:23:43Z

fe/fe-core/src/main/java/com/starrocks/sql/optimizer/transformer/RelationTransformer.java

@@ -214,10 +219,10 @@ public OptExprBuilder visitValues(ValuesRelation node, ExpressionMapping context

    @Override
    public OptExprBuilder visitTable(TableRelation node, ExpressionMapping context) {
-        ImmutableMap.Builder<ColumnRefOperator, Column> columns = ImmutableMap.builder();
+        ImmutableMap.Builder<ColumnRefOperator, Column> colRefToColumnMetaMap = ImmutableMap.builder();
+        ImmutableMap.Builder<Column, ColumnRefOperator> columnToIds = ImmutableMap.builder();


this is not to id, can change a new name

Youngwb · 2021-09-26T13:25:17Z

fe/fe-core/src/main/java/com/starrocks/sql/optimizer/operator/physical/PhysicalOperator.java

@@ -56,8 +56,4 @@ public ColumnRefSet getUsedColumns() {

        return result;
    }
-
-    public abstract boolean equals(Object o);


keep may be better, others may forget implements these method when extends this class

equal has been implemented in the parent class according to predicate and limit, and the subclass needs to implement its own function according to its own class attributes

去掉多余的"它"

HangyuanLiu added 2 commits September 22, 2021 21:33

Delete the inheritance relationship between Pattern and Operator

7fb5349

scan-prune

ab50073

HangyuanLiu force-pushed the logical-scan branch 3 times, most recently from fc707fd to f734302 Compare September 24, 2021 12:18

rebase master

dbe44cd

HangyuanLiu force-pushed the logical-scan branch from f734302 to dbe44cd Compare September 24, 2021 12:45

kangkaisen reviewed Sep 26, 2021

View reviewed changes

Seaven reviewed Sep 26, 2021

View reviewed changes

Seaven previously approved these changes Sep 26, 2021

View reviewed changes

HangyuanLiu dismissed Seaven’s stale review via adc0c4f September 26, 2021 09:09

HangyuanLiu force-pushed the logical-scan branch 3 times, most recently from 5dcc8f1 to 9e4fc26 Compare September 26, 2021 12:21

fix comment

67d1b60

HangyuanLiu force-pushed the logical-scan branch from 9e4fc26 to 67d1b60 Compare September 26, 2021 12:51

Youngwb reviewed Sep 26, 2021

View reviewed changes

rename columnToIds

24d2f66

Seaven approved these changes Sep 27, 2021

View reviewed changes

Youngwb approved these changes Sep 27, 2021

View reviewed changes

kangkaisen merged commit 46285e0 into StarRocks:main Sep 27, 2021

HangyuanLiu deleted the logical-scan branch November 5, 2021 02:12

caneGuy pushed a commit to caneGuy/starrocks that referenced this pull request Mar 28, 2023

Update Materialized_view.md (StarRocks#375)

24672a2

去掉多余的"它"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ScanOperator to eliminate variability #375

Refactor ScanOperator to eliminate variability #375

HangyuanLiu commented Sep 23, 2021

HangyuanLiu commented Sep 24, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

kangkaisen Sep 26, 2021

Seaven Sep 26, 2021 •

edited

Loading

HangyuanLiu Sep 26, 2021

Youngwb Sep 26, 2021

Youngwb Sep 26, 2021 •

edited

Loading

HangyuanLiu Sep 27, 2021

Refactor ScanOperator to eliminate variability #375

Refactor ScanOperator to eliminate variability #375

Conversation

HangyuanLiu commented Sep 23, 2021

HangyuanLiu commented Sep 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seaven Sep 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Youngwb Sep 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seaven Sep 26, 2021 •

edited

Loading

Youngwb Sep 26, 2021 •

edited

Loading