[opt](hive)opt select count(*) stmt push down agg on parquet in hive . #22115

hubgeter · 2023-07-22T13:49:26Z

Proposed changes

Optimization "select count(*) from table" stmtement , push down "count" type to be .

support file type : parquet ，orc in hive .

4kfiles , 60kwline num

before: 1 min 37.70 sec

after: 50.18 sec
50files , 60kwline num

before: 1.12 sec

after: 0.82 sec

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

github-actions · 2023-07-22T13:57:59Z

clang-tidy review says "All clean, LGTM! 👍"

morningman · 2023-07-23T03:36:08Z

be/src/vec/exec/format/parquet/vparquet_reader.cpp

+    //fill one column is enough
+    auto cols = block->mutate_columns();
+    for (auto& col : cols) {
+        col->resize(rows);


The rows maybe too large for the resize?
Normally, a block only return 4096 rows. But here you may return unlimited rows.
I think it should be splitted in batch?

morningman · 2023-07-23T03:38:43Z

be/src/vec/exec/format/generic_reader.h

@@ -31,6 +31,12 @@ class Block;
 class GenericReader {
 public:
    virtual Status get_next_block(Block* block, size_t* read_rows, bool* eof) = 0;
+
+    virtual Status get_next_block(Block* block, size_t* read_rows, bool* eof,


How about merge these 2 methods?

good idea , but i need append parameter TPushAggOp::type push_down_agg_type_opt to all get_next_block functions

morningman · 2023-07-23T13:48:39Z

be/src/vec/exec/scan/vfile_scanner.cpp

+
+            if (_parent->push_down_agg_type_opt != TPushAggOp::type ::NONE) {
+                //Prevent FE  misjudging the "select count/min/max ..." statement
+                if (Status::OK() == _cur_reader->get_next_block(_src_block_ptr, &read_rows,


So if here _cur_reader->get_next_block return error, it will go on calling another get_next_block(), just like a retry?

I suggest that we should make sure FE give the right plan, and here we just use if...else.

Yes, because FE needs to add a lot of redundant code to determine the file type in order to obtain the correct plan 。

morningman · 2023-07-23T13:51:24Z

gensrc/thrift/PlanNodes.thrift

-  15: optional set<i32> output_column_unique_ids
-  16: optional list<i32> distribute_column_ids
-  17: optional i32 schema_version
+  12: optional bool use_topn_opt


You can't modify the origin structure of thrift, or it will cause problem when upgrading.
You can mark the old push_down_agg_type_opt as Deprecated, and make some compatibility
when visiting this field

morningman · 2023-07-23T13:53:26Z

be/src/vec/exec/scan/vscan_node.h

@@ -351,6 +351,9 @@ class VScanNode : public ExecNode, public RuntimeFilterConsumer {
    std::unordered_map<std::string, int> _colname_to_slot_id;
    std::vector<int> _col_distribute_ids;

+public:
+    TPushAggOp::type push_down_agg_type_opt;


Better not using public to define a field

morningman · 2023-07-23T13:57:17Z

fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java

-        if (pushDownAggNoGroupingOp != null) {
-            msg.olap_scan_node.setPushDownAggTypeOpt(pushDownAggNoGroupingOp);
+        if (pushDownAggNoGroupingOp != TPushAggOp.NONE) {
+            msg.setPushDownAggTypeOpt(pushDownAggNoGroupingOp);


I think we can ALWAYS set this field

morningman · 2023-07-23T14:05:13Z

fe/fe-core/src/main/java/org/apache/doris/planner/external/HiveScanNode.java

-        }
+        textParams.setColumnSeparator(hmsTable.getRemoteTable().getSd().getSerdeInfo().getParameters()
+                .getOrDefault(PROP_FIELD_DELIMITER, DEFAULT_FIELD_DELIMITER));
+        textParams.setLineDelimiter(DEFAULT_LINE_DELIMITER);


Why changing this?

I made a mistake 。There's no need to change here。

morningman · 2023-07-23T14:06:01Z

fe/fe-core/src/main/java/org/apache/doris/planner/external/HiveScanNode.java

+        }
+
+        String aggFunctionName = aggExpr.getFnName().getFunction();
+        if (aggFunctionName.equalsIgnoreCase("COUNT") && fileFormatType == TFileFormatType.FORMAT_PARQUET) {


Need to implement orc too

morningman · 2023-07-23T14:06:43Z

fe/fe-core/src/main/java/org/apache/doris/planner/external/HiveScanNode.java

+    }
+
+    @Override
+    public boolean pushDownAggNoGroupingCheckCol(FunctionCallExpr aggExpr, Column col) {


For external table, always return false.

when you use select count(*)statement ,pushDownAggNoGroupingCheckCol will not be executed .
if you use select count(a) statement , pushDownAggNoGroupingCheckCol will check col a.

github-actions · 2023-07-24T17:05:02Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2023-07-24T17:32:58Z

clang-tidy review says "All clean, LGTM! 👍"

hubgeter · 2023-07-24T17:34:57Z

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java

-            return aggregate.withChildren(ImmutableList.of(
-                    new PhysicalStorageLayerAggregate(physicalOlapScan, mergeOp)
-            ));
+            return canNotPush;


if you want

PhysicalOlapScan physicalScan; if (logicalScan instanceof LogicalOlapScan) { physicalScan = (PhysicalOlapScan) new LogicalOlapScanToPhysicalOlapScan() .build() .transform((LogicalOlapScan) logicalScan, cascadesContext) .get(0); } else if (logicalScan instanceof LogicalFileScan) { physicalScan = (PhysicalFileScan) new LogicalFileScanToPhysicalFileScan() .build() .transform((LogicalFileScan) logicalScan, cascadesContext) .get(0); } else { return canNotPush; } if (project != null) { return aggregate.withChildren(ImmutableList.of( project.withChildren( ImmutableList.of(new PhysicalStorageLayerAggregate(physicalScan, mergeOp))) )); } else { return aggregate.withChildren(ImmutableList.of( new PhysicalStorageLayerAggregate(physicalScan, mergeOp) )); }

you will get this:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project fe-core: Compilation failure: Compilation failure: [ERROR] /mnt/datadisk1/changyuwei/doris2/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java:[345,72] incompatible types: org.apache.doris.nereids.trees.plans.physical.PhysicalRelation cannot be converted to org.apache.doris.nereids.trees.plans.physical.PhysicalCatalogRelation [ERROR] /mnt/datadisk1/changyuwei/doris2/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java:[349,51] incompatible types: org.apache.doris.nereids.trees.plans.physical.PhysicalRelation cannot be converted to org.apache.doris.nereids.trees.plans.physical.PhysicalCatalogRelation

😭😤😭

morningman · 2023-07-25T16:17:48Z

be/src/vec/exec/format/orc/vorc_reader.cpp

+            col->resize(rows);
+        }
+
+        *read_rows = rows;


duplicate with line 1388

morningman · 2023-07-25T16:20:44Z

be/src/vec/exec/scan/vscan_node.cpp

+
+    if (tnode.__isset.push_down_agg_type_opt) {
+        _push_down_agg_type = tnode.push_down_agg_type_opt;
+    } else if (tnode.olap_scan_node.__isset.push_down_agg_type_opt) {


Add some comment in code to explain these compatibility work

morningman · 2023-07-25T16:23:16Z

Please add some test cases

github-actions · 2023-07-25T16:55:46Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2023-07-25T17:00:07Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2023-07-26T07:09:35Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2023-07-26T07:19:30Z

clang-tidy review says "All clean, LGTM! 👍"

morningman

LGTM

github-actions · 2023-07-27T01:37:23Z

PR approved by at least one committer and no changes requested.

github-actions · 2023-07-27T01:37:25Z

PR approved by anyone and no changes requested.

morningman · 2023-07-27T05:59:59Z

run buildall

hubgeter · 2023-07-28T03:20:25Z

run buildall

github-actions · 2023-07-28T03:23:20Z

clang-tidy review says "All clean, LGTM! 👍"

hello-stephen · 2023-07-28T04:18:48Z

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 46.36 seconds
stream load tsv: 505 seconds loaded 74807831229 Bytes, about 141 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 64 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 30 seconds loaded 861443392 Bytes, about 27 MB/s
insert into select: 29.1 seconds inserted 10000000 Rows, about 343K ops/s
storage size: 17156006903 Bytes

morningman

LGTM

github-actions · 2023-07-28T09:28:04Z

PR approved by at least one committer and no changes requested.

kaka11chen

LGTM

apache#22115) Optimization "select count(*) from table" stmtement , push down "count" type to BE. support file type : parquet ，orc in hive . 1. 4kfiles , 60kwline num before: 1 min 37.70 sec after: 50.18 sec 2. 50files , 60kwline num before: 1.12 sec after: 0.82 sec

#22115) Optimization "select count(*) from table" stmtement , push down "count" type to BE. support file type : parquet ，orc in hive . 1. 4kfiles , 60kwline num before: 1 min 37.70 sec after: 50.18 sec 2. 50files , 60kwline num before: 1.12 sec after: 0.82 sec

apache#22115) Optimization "select count(*) from table" stmtement , push down "count" type to BE. support file type : parquet ，orc in hive . 1. 4kfiles , 60kwline num before: 1 min 37.70 sec after: 50.18 sec 2. 50files , 60kwline num before: 1.12 sec after: 0.82 sec

… with only one column. (#25222) after pr #22115 . Fixed the bug that when selecting count(*) from table, if the table has only one column, the aggregate count is not pushed down.

… with only one column. (apache#25222) after pr apache#22115 . Fixed the bug that when selecting count(*) from table, if the table has only one column, the aggregate count is not pushed down.

morningman reviewed Jul 23, 2023

View reviewed changes

hubgeter force-pushed the select_count branch from 825af66 to d7421d6 Compare July 24, 2023 16:57

hubgeter commented Jul 24, 2023

View reviewed changes

morningman reviewed Jul 25, 2023

View reviewed changes

hubgeter force-pushed the select_count branch from b4c3ee1 to 679102a Compare July 25, 2023 16:49

hubgeter force-pushed the select_count branch from 339cb69 to 3800417 Compare July 26, 2023 07:01

morningman previously approved these changes Jul 27, 2023

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 27, 2023

github-actions bot added the reviewed label Jul 27, 2023

924060929 previously approved these changes Jul 27, 2023

View reviewed changes

hubgeter added 9 commits July 28, 2023 11:16

[opt](hive)opt select count(*) stmt push down agg on hive

d5e00d0

[opt](hive)opt select count(*) stmt push down agg on hive

e8a593f

[opt](hive)opt select count(*) stmt push down agg on hive

16312f0

[opt](hive)opt select count(*) stmt push down agg on hive

686fabc

[opt](hive)opt select count(*) stmt push down agg on hive

b53dc2d

[opt](hive)opt select count(*) stmt push down agg on hive

ca0f80f

[opt](hive)opt select count(*) stmt push down agg on parquet in hive .

886440d

[opt](hive)opt select count(*) stmt push down agg on parquet in hive .

83fc5dc

[opt](hive)opt select count(*) stmt push down agg on parquet in hive .

28317dc

[opt](hive)opt select count(*) stmt push down agg on parquet in hive .

0d9e4bf

hubgeter dismissed stale reviews from 924060929 and morningman via 0d9e4bf July 28, 2023 03:16

hubgeter force-pushed the select_count branch from 30cd822 to 0d9e4bf Compare July 28, 2023 03:16

github-actions bot removed the approved Indicates a PR has been approved by one committer. label Jul 28, 2023

morningman approved these changes Jul 28, 2023

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 28, 2023

hubgeter requested a review from 924060929 July 28, 2023 09:51

kaka11chen approved these changes Jul 28, 2023

View reviewed changes

morningman merged commit ae8a263 into apache:master Jul 28, 2023

morningman added the dev/2.0.1 label Jul 28, 2023

xiaokang added merge_conflict dev/2.0.1-merged and removed dev/2.0.1 labels Aug 8, 2023

hubgeter mentioned this pull request Oct 10, 2023

[fix](Nereids)Fix the bug that count(*) does not push down for tables with only one column. #25222

Merged

[opt](hive)opt select count(*) stmt push down agg on parquet in hive . #22115

[opt](hive)opt select count(*) stmt push down agg on parquet in hive . #22115

Conversation

hubgeter commented Jul 22, 2023 • edited Loading

Proposed changes

Further comments

github-actions bot commented Jul 22, 2023

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 24, 2023

github-actions bot commented Jul 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morningman commented Jul 25, 2023

github-actions bot commented Jul 25, 2023

github-actions bot commented Jul 25, 2023

github-actions bot commented Jul 26, 2023

github-actions bot commented Jul 26, 2023

morningman left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 27, 2023

github-actions bot commented Jul 27, 2023

morningman commented Jul 27, 2023

hubgeter commented Jul 28, 2023

github-actions bot commented Jul 28, 2023

hello-stephen commented Jul 28, 2023

morningman left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 28, 2023

kaka11chen left a comment

Choose a reason for hiding this comment

hubgeter commented Jul 22, 2023 •

edited

Loading