You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2023/12/15 10:23:16 UTC

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20804


Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls. Then each file only needs to
be processed once. The planner is also modified to generate only one
scan range per file.

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
3 files changed, 68 insertions(+), 35 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/1
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 3:

> Patch Set 3:
> 
> So I think using FileMetaData.num_rows for count star optimizations should be acceptable.

Thanks for pointing that out. Looks like it should be safe now since ZeroSlotTableScan has use FileMetaData.num_rows already.

Since this will be a behavior change, do you mind adding a backend flag to control this? Default to count using FileMetaData.num_rows, but back to RowGroups.num_rows when flag is disabled. This way, user can revert to old behavior if they do hit an inaccurate FileMetaData.num_rows issue.

Basic performance benchmark is also desirable to ensure no regression happen like IMPALA-11123. Maybe you can steal TPCDS-Q_COUNT_OPTIMIZED, TPCDS-Q_COUNT_UNOPTIMIZED, and TPCDS-Q_COUNT_ZERO_SLOT from https://gerrit.cloudera.org/c/19927 and run single_node_perf_run.py such as:

./bin/single_node_perf_run.py --num_impalads=3 \
    --workloads=tpcds --iterations=9 --table_formats=parquet/none/none \
    --query_names=TPCDS-Q_COUNT_OPTIMIZED,TPCDS-Q_COUNT_UNOPTIMIZED,TPCDS-Q_COUNT_ZERO_SLOT \
    asf-master <this_patch_branch>

Even better if you can do it with larger scale TPC-DS like 10GB:

./bin/single_node_perf_run.py --num_impalads=3 --load --scale=10 \
    --workloads=tpcds --iterations=9 --table_formats=parquet/none/none \
    --query_names=TPCDS-Q_COUNT_OPTIMIZED,TPCDS-Q_COUNT_UNOPTIMIZED,TPCDS-Q_COUNT_ZERO_SLOT \
    asf-master <this_patch_branch>

Using tpch_parquet.lineitem should be fine as well.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 20:07:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10150/


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Jan 2024 09:56:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14986/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jan 2024 09:51:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 3:

> Since this will be a behavior change, do you mind adding a backend flag to control this? Default to count using FileMetaData.num_rows, but back to RowGroups.num_rows when flag is disabled. This way, user can revert to old behavior if they do hit an inaccurate FileMetaData.num_rows issue.
> 
> Basic performance benchmark is also desirable to ensure no regression happen like IMPALA-11123. Maybe you can steal TPCDS-Q_COUNT_OPTIMIZED, TPCDS-Q_COUNT_UNOPTIMIZED, and TPCDS-Q_COUNT_ZERO_SLOT from https://gerrit.cloudera.org/c/19927 and run single_node_perf_run.py such as:
> 
> ./bin/single_node_perf_run.py --num_impalads=3 \
>     --workloads=tpcds --iterations=9 --table_formats=parquet/none/none \
>     --query_names=TPCDS-Q_COUNT_OPTIMIZED,TPCDS-Q_COUNT_UNOPTIMIZED,TPCDS-Q_COUNT_ZERO_SLOT \
>     asf-master <this_patch_branch>
> 
> Even better if you can do it with larger scale TPC-DS like 10GB:
> 
> ./bin/single_node_perf_run.py --num_impalads=3 --load --scale=10 \
>     --workloads=tpcds --iterations=9 --table_formats=parquet/none/none \
>     --query_names=TPCDS-Q_COUNT_OPTIMIZED,TPCDS-Q_COUNT_UNOPTIMIZED,TPCDS-Q_COUNT_ZERO_SLOT \
>     asf-master <this_patch_branch>
> 
> Using tpch_parquet.lineitem should be fine as well.

Thanks for the guidance! I'll try to add a backend flag and do some performance tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 09:33:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14834/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 25 Dec 2023 11:52:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2024 19:11:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 13:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15017/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 13
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Jan 2024 09:24:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14821/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 06:30:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 7:

(6 comments)

I will back to review this after new year break, and maybe help run benchmark at larger scale.
In the meantime, I have few comments.

http://gerrit.cloudera.org:8080/#/c/20804/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20804/5//COMMIT_MSG@30
PS5, Line 30: TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16
> I checked profiles without this patch and found that 'NumFileMetadataRead' 
Make sense.


http://gerrit.cloudera.org:8080/#/c/20804/7/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@480
PS7, Line 480:       *dst_slot = file_metadata_.row_groups[row_group_idx_].num_rows;
Is it possible to unify the code here?
If parquet_count_star_use_file_metadata == true, set capacity = 1 and *dst_slot = file_metadata_.num_rows.


http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/service/query-options.cc
File be/src/service/query-options.cc:

http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/service/query-options.cc@1198
PS5, Line 1198:       case TImpalaQueryOptions::PARQUET_COUNT_STAR_USE_FILE_METADATA: {
              :         query_options->__set_parquet_count_star_use_file_metadata(IsTrue(value));
              :         break;
              :       }
> Fixed
Done


http://gerrit.cloudera.org:8080/#/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@426
PS7, Line 426:     for (FeFsPartition partition: partitions_) {
             :       if (partition.getFileFormat() != HdfsFileFormat.ICEBERG) {
             :         fileFormats_.add(partition.getFileFormat());
             :       }
             :     }
             : 
             :     if (canApplyCountStarOptimization(analyzer, fileFormats_)) {
             :       Preconditions.checkState(desc_.getPath().destTable() != null);
             :       Preconditions.checkState(collectionConjuncts_.isEmpty());
             :       countStarSlot_ = applyCountStarOptimization(analyzer);
             :     }
Is it OK to move these into computeScanRangeLocations?
Note that if sampledFiles_ is not null, the original code might skip a partition and not adding it to fileFormats_.

Maybe you can reorganize the partitions_ loop in computeScanRangeLocations to filter based on sampledFiles_ first.


http://gerrit.cloudera.org:8080/#/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1396
PS7, Line 1396: (countStarSlot_ != null
              :             && analyzer.getQueryOptions().parquet_count_star_use_file_metadata)
Contain this into a boolean variable. Possibly at caller of transformBlocksToScanRanges()?


http://gerrit.cloudera.org:8080/#/c/20804/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test:

http://gerrit.cloudera.org:8080/#/c/20804/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test@10
PS7, Line 10: ---- RUNTIME_PROFILE
            : aggregation(SUM, NumRowGroups): 0
            : aggregation(SUM, NumFileMetadataRead): 24
            : aggregation(SUM, RowsRead): 0
For modified tests here, can you also add the same test with parquet_count_star_use_file_metadata == false?



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 18:28:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#8).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg-default.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/query_test/test_aggregation.py
12 files changed, 316 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/8
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10246/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2024 14:41:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zoltan Borok-Nagy, Zihao Ye, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#15).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Before this patch frontend generates multiple scan ranges for a
parquet file when count star optimization is enabled. Backend function
HdfsParquetScanner::GetNextInternal() also call NextRowGroup()
multiple times to find row groups and sum up RowGroup.num_rows. This
could be inefficient because we only need to read file metadata to
compute count star. This patch optimizes it by creating only one
scan range that contains the file footer for each parquet file.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table are generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/hdfs-tiny-scan.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-in-predicate-push-down.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/util/parse_util.py
11 files changed, 138 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/15
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zoltan Borok-Nagy, Zihao Ye, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#12).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/hdfs-tiny-scan.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-in-predicate-push-down.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/query_test/test_aggregation.py
M tests/util/parse_util.py
16 files changed, 172 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/12
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14737/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 10:51:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 5:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/14810/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 12:51:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 13:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10192/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 13
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 01:14:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10237/


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 08:58:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: dst_tuple
> Is it okay if we don't update dst_tuple inside the loop? If there are multi
Yes, it is. Since 'dst_slot' points to the slot of the 'dst_tuple', we only need to update 'dst_slot' in the loop.


http://gerrit.cloudera.org:8080/#/c/20804/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1274
PS8, Line 1274: isFooterOnly
> nit: This parameter seems unnecessary. Can't we do the same check (countSta
This is to address the review feedback in PS7:https://gerrit.cloudera.org/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java#1397. 

I think wrapping it into a boolean variable makes codes in transformBlocksToScanRanges() more readable.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 05 Jan 2024 05:09:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10237/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 04:25:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15164/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 04:45:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zihao Ye, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#9).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg-default.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/query_test/test_aggregation.py
12 files changed, 316 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/9
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 3:

> Patch Set 3:
> 
> I think there is a strong reason why Impala trust RowGroups.num_rows more than FileMetaData.num_rows. Maybe there are still invalid parquet files out there that is written with mismatched FileMetaData.num_rows.
> 
> See:
> https://issues.apache.org/jira/browse/IMPALA-3943
> https://issues.apache.org/jira/browse/IMPALA-2230

Thanks for your reply, Riza!
I investigated the issues, especially IMPALA-3943. For parquet scans, Impala now treats the file with FileMetaData.num_rows=0 as an empty file, see: https://github.com/apache/impala/blob/4114fe8db6ec80b2e1679e946555f91ab7043f2e/be/src/exec/parquet/hdfs-parquet-scanner.cc#L895C1-L898. And I also found other places that using FileMetaData.num_rows to generate query results for metadata only queries, see: https://github.com/apache/impala/blob/4114fe8db6ec80b2e1679e946555f91ab7043f2e/be/src/exec/parquet/hdfs-parquet-scanner.cc#L477-L481. Besides, Impala-3943 also added warning logs for inconsistency of FileMetaData.num_rows and RowGroups.num_rows.

So I think using FileMetaData.num_rows for count star optimizations should be acceptable.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 09:56:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@425
PS6, Line 425:     
line has trailing whitespace



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 06:02:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#4).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
10 files changed, 124 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/4
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#5).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
10 files changed, 124 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/5
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10150/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Jan 2024 05:37:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 12: Code-Review+1

(2 comments)

Thanks for working on this!

http://gerrit.cloudera.org:8080/#/c/20804/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20804/12//COMMIT_MSG@16
PS12, Line 16: A new query option parquet_count_star_use_file_metadata is added for
             : forward compatibility. Its default value is true, if any inconsistency
             : between FileMetaData.num_rows and RowGroup.num_rows is found, we can
             : set it to false to get same results as old versions.
Probably that would be a corrupt Parquet file. But if we are afraid of inconsistency, then this solution could also just sum RowGroup.num_rows. I don't think that would have any effect on perf, and there would be no change in behavior. The real perf gain comes from not creating multiple scan ranges for Parquet files.


http://gerrit.cloudera.org:8080/#/c/20804/12/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/12/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1452
PS12, Line 1452:           if (isFooterOnly) {
               :             // Only generate one scan range for footer only scans.
               :             currentOffset += remainingLength - currentLength;
               :             remainingLength = currentLength;
               :           }
Why do we need to this now? We didn't do that for partition key scans.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jan 2024 15:31:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: 
> The following query return different output with and without PARQUET_COUNT_
It turned out all 'dst_row' points to the same 'dst_tuple', so we can't reuse the same 'tuple_buf' to hold different values. I have updated this and added new test cases.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jan 2024 13:09:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14931/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jan 2024 13:34:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: dst_tuple
Is it okay if we don't update dst_tuple inside the loop? If there are multiple row groups, this could lead to incorrect results.


http://gerrit.cloudera.org:8080/#/c/20804/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1274
PS8, Line 1274: isFooterOnly
nit: This parameter seems unnecessary. Can't we do the same check (countStarSlot_ != null && analyzer.getQueryOptions().parquet_count_star_use_file_metadata) in transformBlocksToScanRanges()?



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Thu, 04 Jan 2024 07:55:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14739/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 13:12:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Before this patch frontend generates multiple scan ranges for a
parquet file when count star optimization is enabled. Backend function
HdfsParquetScanner::GetNextInternal() also call NextRowGroup()
multiple times to find row groups and sum up RowGroup.num_rows. This
could be inefficient because we only need to read file metadata to
compute count star. This patch optimizes it by creating only one
scan range that contains the file footer for each parquet file.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table are generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Reviewed-on: http://gerrit.cloudera.org:8080/20804
Reviewed-by: Zoltan Borok-Nagy <bo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/hdfs-tiny-scan.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-in-predicate-push-down.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/util/parse_util.py
11 files changed, 138 insertions(+), 63 deletions(-)

Approvals:
  Zoltan Borok-Nagy: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 16
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 3:

I think there is a strong reason why Impala trust RowGroups.num_rows more than FileMetaData.num_rows. Maybe there are still invalid parquet files out there that is written with mismatched FileMetaData.num_rows.

See:
https://issues.apache.org/jira/browse/IMPALA-3943
https://issues.apache.org/jira/browse/IMPALA-2230


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sun, 17 Dec 2023 07:30:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 12:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20804/11/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/11/be/src/exec/parquet/hdfs-parquet-scanner.cc@445
PS11, Line 445: _file_metadata) {
> What is the reason behind this check?
Done.

When max_scan_range_length is set to a small value, we may generate more than one scan range per block. We should also handle the cases. Related checks are updated in frontend.


http://gerrit.cloudera.org:8080/#/c/20804/11/be/src/exec/parquet/hdfs-parquet-scanner.cc@447
PS11, Line 447: capacity = 1;
> Add this check before assignment:
Done


http://gerrit.cloudera.org:8080/#/c/20804/11/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/11/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1260
PS11, Line 1260: If parquet_count_star_use_file_metadata is enabled, we only need the
               :           // 'num_ro
> Mention the difference between enabling/disabling parquet_count_star_use_fi
Done


http://gerrit.cloudera.org:8080/#/c/20804/11/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:

http://gerrit.cloudera.org:8080/#/c/20804/11/tests/query_test/test_aggregation.py@271
PS11, Line 271: vector.get_value('exec_option')['batch_size'] = 1
              :     self.run_test_case('QueryTest/parquet-stats-agg', vector, unique_database)
              : 
              :     vector.get_value('exec_option')['parquet_count_star_
> If parquet_count_star_use_file_metadata = true becomes default, I'd prefer 
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jan 2024 09:25:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#6).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
10 files changed, 119 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/6
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14809/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 12:55:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14822/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 06:32:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: dst_tuple
> I still think there's an issue here. Only tuple_buf is updated in the loop,
There is already a test case covering this, see 'QueryTest/parquet-stats-agg', we query multiblock tables including 'functional_parquet.lineitem_multiblock' and 'functional_parquet.lineitem_sixblocks'. I have tested it with this patch and got the correct results.

I didn't quite understand what you meant. But 'So, in each iteration, data is written to the first Tuple' is not true. We created a new 'dst_row' in the loop(see L470).



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 05 Jan 2024 07:07:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zoltan Borok-Nagy, Zihao Ye, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#10).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg-default.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/query_test/test_aggregation.py
12 files changed, 327 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/10
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 10
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14883/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 05 Jan 2024 05:36:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: dst_tuple
> There is already a test case covering this, see 'QueryTest/parquet-stats-ag
I have reviewed the parquet files for 'functional_parquet.lineitem_multiblock' and 'functional_parquet.lineitem_sixblocks'. They both indeed have multiple row groups, each containing 100 rows, which doesn't replicate the issue. Therefore, I created a parquet file with three row groups, each containing 1, 2, and 3 rows respectively. After disabling PARQUET_COUNT_STAR_USE_FILE_METADATA and reading it, I expected the count(*) to be 6 (1 + 2 + 3), but I actually obtained 9 (3 * 3), consistent with my previous assumption. Although we create a new dst_row within the loop, the SetTuple function always receives an unchanged dst_tuple, meaning all TupleRows reference the same Tuple that has not been updated. This Tuple is then written with a value of 3 during the final iteration, causing all three TupleRows to reference a Tuple with the content of 3, leading to an erroneous result of 3 * 3 = 9.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jan 2024 10:03:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#3).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in Parquet 'RowGroup.num_rows' field to compute count star,
it still needs to find row groups and sum all 'RowGroup.num_rows'.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls. Then each file only needs to
be processed once. The planner is also modified to generate only one
scan range per file.

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
3 files changed, 70 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/3
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20804/12//COMMIT_MSG@16
PS12, Line 16: A new query option parquet_count_star_use_file_metadata is added for
             : forward compatibility. Its default value is true, if any inconsistency
             : between FileMetaData.num_rows and RowGroup.num_rows is found, we can
             : set it to false to get same results as old versions.
> Yeah. I adjusted it to sum RowGroup.num_rows in PS13 and got the same perfo
Yes, I think now it makes sense to remove the query option.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 17:48:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20804/12/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:

http://gerrit.cloudera.org:8080/#/c/20804/12/tests/query_test/test_aggregation.py@275
PS12, Line 275: )
flake8: E501 line too long (91 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/20804/12/tests/query_test/test_aggregation.py@277
PS12, Line 277: )
flake8: E501 line too long (91 > 90 characters)



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 12
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jan 2024 09:25:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 11:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10154/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jan 2024 03:10:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 11: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10154/


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jan 2024 07:31:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/14954/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jan 2024 12:56:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20804/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20804/5//COMMIT_MSG@30
PS5, Line 30: TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16
Any idea why this benchmark does not improve? I thought this patch should improve optimized count star.


http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/exec/parquet/hdfs-parquet-scanner.cc@464
PS5, Line 464:     int capacity =
             :         min(static_cast<int>(file_metadata_.row_groups.size()), row_batch->capacity());
             :     RETURN_IF_ERROR(
             :         RowBatch::ResizeAndAllocateTupleBuffer(state_, row_batch->tuple_data_pool(),
             :             row_batch->row_desc()->GetRowSize(), &capacity, &tuple_buf_size, &tuple_buf));
Unnecessary changes?


http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/service/query-options.cc
File be/src/service/query-options.cc:

http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/service/query-options.cc@1198
PS5, Line 1198:       case TImpalaQueryOptions::c: {
              :         query_options->__set_parquet_count_star_use_file_metadata(IsTrue(value));
              :         break;
              :       }
Option parsing does not look right to me.


http://gerrit.cloudera.org:8080/#/c/20804/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@a411
PS5, Line 411: 
I think canApplyCountStarOptimization should not change, especially this check on fileFormats.
There is a special handling during file format detection. See fileFormats_.add() in computeScanRangeLocations().



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 21:35:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zoltan Borok-Nagy, Zihao Ye, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#13).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/hdfs-tiny-scan.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-in-predicate-push-down.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/query_test/test_aggregation.py
M tests/util/parse_util.py
16 files changed, 177 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/13
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 13
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zihao Ye (Code Review)" <ge...@cloudera.org>.
Zihao Ye has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: dst_tuple
> Yes, it is. Since 'dst_slot' points to the slot of the 'dst_tuple', we only
I still think there's an issue here. Only tuple_buf is updated in the loop, but neither dst_tuple nor dst_slot is updated to point to the new position. So, in each iteration, data is written to the first Tuple, and each TupleRow's SetTuple() references this Tuple. Therefore, for a file with n row groups, the final scan result would be n times the number of rows in the last row group, rather than the correct sum of rows from each row group. I'm not entirely sure if the existing test files have enough row groups. Perhaps we could add a test case with multiple row groups, each with a different number of rows, to confirm this issue?



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Comment-Date: Fri, 05 Jan 2024 06:42:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 5:

I chose to add a query option instead of a backend flag to control whether to enable this optimization. The reason is:
- This patch also contains changes on frontend.
- Different configurations for backends in a cluster can lead to incorrect query results.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 12:35:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20804/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20804/5//COMMIT_MSG@30
PS5, Line 30: TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16
> Any idea why this benchmark does not improve? I thought this patch should i
I checked profiles without this patch and found that 'NumFileMetadataRead' is the same as 'NumRowGroups'. In this case, the calculation cost is not saved.


http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/exec/parquet/hdfs-parquet-scanner.cc@464
PS5, Line 464:     int capacity = min(
             :         static_cast<int>(file_metadata_.row_groups.size()), row_batch->capacity());
             :     RETURN_IF_ERROR(RowBatch::ResizeAndAllocateTupleBuffer(state_,
             :         row_batch->tuple_data_pool(), row_batch->row_desc()->GetRowSize(),
             :         &capacity, &tuple_buf_size, &tuple_buf));
> Unnecessary changes?
Done


http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/service/query-options.cc
File be/src/service/query-options.cc:

http://gerrit.cloudera.org:8080/#/c/20804/5/be/src/service/query-options.cc@1198
PS5, Line 1198:       case TImpalaQueryOptions::PARQUET_COUNT_STAR_USE_FILE_METADATA: {
              :         query_options->__set_parquet_count_star_use_file_metadata(IsTrue(value));
              :         break;
              :       }
> Option parsing does not look right to me.
Fixed


http://gerrit.cloudera.org:8080/#/c/20804/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@a411
PS5, Line 411: 
> I think canApplyCountStarOptimization should not change, especially this ch
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 22 Dec 2023 06:12:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 13: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10192/


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 13
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jan 2024 05:23:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 11:

(4 comments)

The idea is good, and it does save some resources during query execution since the new behavior will only schedule 1 scan range per file.

But it does change behavior when file_metadata_.num_rows > 0 and file_metadata_.num_rows != sum(row_group.num_rows). I will defer to Csaba or Zoltan about enabling it by default or not.

http://gerrit.cloudera.org:8080/#/c/20804/11/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/11/be/src/exec/parquet/hdfs-parquet-scanner.cc@445
PS11, Line 445: state_->query_options().max_scan_range_length <= 0
What is the reason behind this check?


http://gerrit.cloudera.org:8080/#/c/20804/11/be/src/exec/parquet/hdfs-parquet-scanner.cc@447
PS11, Line 447: capacity = 1;
Add this check before assignment:

DCHECK_GE(capacity, 1);


http://gerrit.cloudera.org:8080/#/c/20804/11/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/11/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1260
PS11, Line 1260: For optimizing count star scans, we only need the block containing file
               :           // footer.
Mention the difference between enabling/disabling parquet_count_star_use_file_metadata option.


http://gerrit.cloudera.org:8080/#/c/20804/11/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:

http://gerrit.cloudera.org:8080/#/c/20804/11/tests/query_test/test_aggregation.py@271
PS11, Line 271: self.run_test_case('QueryTest/parquet-stats-agg-default', vector, unique_database)
              :     vector.get_value('exec_option')['parquet_count_star_use_file_metadata'] = 'false'
              :     self.run_test_case('QueryTest/parquet-stats-agg', vector, unique_database)
              :     vector.get_value('exec_option')['batch_size'] = 1024
If parquet_count_star_use_file_metadata = true becomes default, I'd prefer the new test file to test the old behavior instead. So like:

parquet_count_star_use_file_metadata = true : parquet-stats-agg.test
parquet_count_star_use_file_metadata = false : parquet-stats-agg-old-behavior.test

Also, please make sure both test files contain the exact same test cases.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jan 2024 18:19:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/8/be/src/exec/parquet/hdfs-parquet-scanner.cc@473
PS8, Line 473: dst_tuple
> I have reviewed the parquet files for 'functional_parquet.lineitem_multiblo
The following query return different output with and without PARQUET_COUNT_STAR_USE_FILE_METADATA enabled:

select count(*) from functional_parquet.lineitem_multiblock_variable_num_rows;



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jan 2024 04:40:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 14:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10234/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 14
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 04 Feb 2024 10:04:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zoltan Borok-Nagy, Zihao Ye, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#11).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
A testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg-default.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/query_test/test_aggregation.py
13 files changed, 331 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/11
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 11
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Zoltan Borok-Nagy, Zihao Ye, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#14).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Before this patch frontend generates multiple scan ranges for a
parquet file when count star optimization is enabled. Backend function
HdfsParquetScanner::GetNextInternal() also call NextRowGroup()
multiple times to find row groups and sum up RowGroup.num_rows. This
could be inefficient because we only need to read file metadata to
compute count star. This patch optimizes it by creating only one
scan range that contains the file footer for each parquet file.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table are generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Added new test cases to query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/hdfs-tiny-scan.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-in-predicate-push-down.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
M tests/util/parse_util.py
11 files changed, 144 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/14
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 14
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 14:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15162/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 14
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 04 Feb 2024 10:00:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 14: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10234/


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 14
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sun, 04 Feb 2024 14:40:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15: Code-Review+2

Thanks for working on this, LGTM!


-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2024 15:13:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20804/15/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/15/be/src/exec/parquet/hdfs-parquet-scanner.cc@444
PS15, Line 444:     if (file_metadata_.num_rows > 0) {
Add this for backward compatibility: See L893 in this file.



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 15
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Feb 2024 04:24:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 13:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20804/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20804/12//COMMIT_MSG@16
PS12, Line 16: A new query option parquet_count_star_use_file_metadata is added for
             : forward compatibility. Its default value is true, if any inconsistency
             : between FileMetaData.num_rows and RowGroup.num_rows is found, we can
             : set it to false to get same results as old versions.
> Probably that would be a corrupt Parquet file. But if we are afraid of inco
Yeah. I adjusted it to sum RowGroup.num_rows in PS13 and got the same performance improvement by running the single node perf test.

Then I think we do not need to introduce this new query option since no behavior changes are made. What do you think?


http://gerrit.cloudera.org:8080/#/c/20804/12/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/12/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1452
PS12, Line 1452:           if (isFooterOnly) {
               :             // Only generate one scan range for footer only scans.
               :             currentOffset += remainingLength - currentLength;
               :             remainingLength = currentLength;
               :           }
> Why do we need to this now? We didn't do that for partition key scans.
For count star optimization scans, it's not a zero-slot scan, we have one slot for num rows statistic. But a partition scan is a zero-slot scan. We create a footer range for every scan range if it is not a zero-slot scan in HdfsScanner::IssueFooterRanges().


http://gerrit.cloudera.org:8080/#/c/20804/12/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:

http://gerrit.cloudera.org:8080/#/c/20804/12/tests/query_test/test_aggregation.py@275
PS12, Line 275: 
> flake8: E501 line too long (91 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/20804/12/tests/query_test/test_aggregation.py@277
PS12, Line 277: 
> flake8: E501 line too long (91 > 90 characters)
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 13
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Zihao Ye <ey...@163.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Jan 2024 09:02:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#2).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls. Then each file only needs to
be processed once. The planner is also modified to generate only one
scan range per file.

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
3 files changed, 70 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/2
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/20804

to look at the new patch set (#7).

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................

IMPALA-12631: Improve count star performance for parquet scans

Backend function HdfsParquetScanner::GetNextInternal() uses the data
stored in the Parquet RowGroup.num_rows field to compute count star,
it still needs to find row groups and sum all RowGroup.num_rows.
This patch uses the 'num_rows' field in Parquet file metadata, it
avoids NextRowGroup() function calls, generates and processes only one
footer range per file.

A new query option parquet_count_star_use_file_metadata is added for
forward compatibility. Its default value is true, if any inconsistency
between FileMetaData.num_rows and RowGroup.num_rows is found, we can
set it to false to get same results as old versions.

The following table shows a performance comparison before and after
the patch. primitive_count_star_multiblock query is a modified
primitive_count_star query that targets a multi-block
tpch10_parquet.lineitem table. The files of the table is generated
by the command `hdfs dfs -Ddfs.block.size=1048576 -cp -f -d`.

+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| Workload          | Query                           | File Format           | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%)  | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval   |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+
| TPCDS(10)         | TPCDS-Q_COUNT_OPTIMIZED         | parquet / none / none | 0.17   | 0.16        |   +2.58%   | * 29.53% * | * 27.16% *     | 30    |   +1.20%       | 0.58    | 0.35   |
| TPCDS(10)         | TPCDS-Q_COUNT_UNOPTIMIZED       | parquet / none / none | 0.27   | 0.26        |   +2.96%   |   8.97%    |   9.94%        | 30    |   +0.16%       | 0.44    | 1.19   |
| TPCDS(10)         | TPCDS-Q_COUNT_ZERO_SLOT         | parquet / none / none | 0.18   | 0.18        |   -0.69%   |   1.65%    |   1.99%        | 30    |   -0.34%       | -1.55   | -1.47  |
| TARGETED-PERF(10) | primitive_count_star_multiblock | parquet / none / none | 0.06   | 0.12        | I -49.88%  |   4.11%    |   3.53%        | 30    | I -99.97%      | -6.54   | -66.81 |
+-------------------+---------------------------------+-----------------------+--------+-------------+------------+------------+----------------+-------+----------------+---------+--------+

Testing:
- Ran PlannerTest#testParquetStatsAgg
- Ran query_test/test_aggregation.py

Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
---
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_optimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_unoptimized.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q_count_zero_slot.test
10 files changed, 119 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/20804/7
-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 7
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>

[Impala-ASF-CR] IMPALA-12631: Improve count star performance for parquet scans

Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20804 )

Change subject: IMPALA-12631: Improve count star performance for parquet scans
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20804/7/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20804/7/be/src/exec/parquet/hdfs-parquet-scanner.cc@480
PS7, Line 480:     // There are no materialized slots and we are not optimizing coun
> Is it possible to unify the code here?
Done


http://gerrit.cloudera.org:8080/#/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@426
PS7, Line 426:     // compute scan range locations with optional sampling
             :     computeScanRangeLocations(analyzer);
             : 
             :     if (hasParquet(fileFormats_)) {
             :       // Compute min-max conjuncts only if the PARQUET_READ_STATISTICS query option is
             :       // set to true.
             :       if (analyzer.getQueryOptions().parquet_read_statistics) {
             :         computeStatsTupleAndConjuncts(analyzer);
             :       }
             :       // Compute dictionary conjuncts only if the PARQUET_DICTIONARY_FILTERING query
             :      
> Is it OK to move these into computeScanRangeLocations?
Done


http://gerrit.cloudera.org:8080/#/c/20804/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1396
PS7, Line 1396: 
              :    * Given a fileDesc of partition, transforms the blocks into TScanRanges. Eac
> Contain this into a boolean variable. Possibly at caller of transformBlocks
Done


http://gerrit.cloudera.org:8080/#/c/20804/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test:

http://gerrit.cloudera.org:8080/#/c/20804/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats-agg.test@10
PS7, Line 10: ---- RUNTIME_PROFILE
            : aggregation(SUM, NumRowGroups): 24
            : aggregation(SUM, NumFileMetadataRead): 24
            : aggregation(SUM, RowsRead): 0
> For modified tests here, can you also add the same test with parquet_count_
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/20804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9cd2448fe51a420d4559d0cc861c4d30822f4fd
Gerrit-Change-Number: 20804
Gerrit-PatchSet: 8
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Mon, 25 Dec 2023 11:28:11 +0000
Gerrit-HasComments: Yes