Hello Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-14071: Refactor 
MathUtil.saturatingMultiplyCardinalities()
......................................................................

IMPALA-14071: Refactor MathUtil.saturatingMultiplyCardinalities()

There are multiple ways to do cardinality multipication that also avoid
integer overflow. Some helper methods available are:
- MathUtil.saturatingMultiplyCardinalities()
- PlanNode.checkedMultiply()
- LongMath.saturatedMultiply()

This patch intent to simplify things by:
- MathUtil.saturatingMultiplyCardinalities() with
  PlanNode.checkedMultiply() into MathUtil.multiplyCardinalities().
- MathUtil.saturatingAddCardinalities() with
  PlanNode.checkedAdd() into MathUtil.addCardinalities().
- Move PlanNode.smallestValidCardinality() to MathUtil.
- Make MathUtil.saturatingMultiply() and MathUtil.saturatingAdd() simply
  a wrapper for LongMath.saturatedMultiply() and LongMath.saturatedAdd()
  accordingly.

multiplyCardinalities(), addCardinalities(), and
smallestValidCardinality() have cardinality Preconditions check.

Harden cardinality calculation in several places by using
multiplyCardinalities() and addCardinalities() accordingly. Added sanity
check PlanNode.verifyCardinality() that is evaluated at the end of
PlanNode.computeStats(). This ensure that cardinality_ and
inputCardinality_ is always valid after PlanNode.computeStats().

Also fixed bug in ExchangeNode.estimateTotalQueueByteSize() that prevent
calculation against negative cardinality or non-positive num nodes.

Testing:
Pass Following FE and EE tests:
CardinalityTest
MathUtilTest
PlannerTest#testSpillableBufferSizing+testResourceRequirements
TpcdsCpuCostPlannerTest
TpcdsPlannerTest
metadata/test_explain.py

Change-Id: I505ab11cfa1024feb4ceac4cffe9c3283be228ce
---
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/IcebergDeleteNode.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/util/MathUtil.java
M fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
M fe/src/test/java/org/apache/impala/util/MathUtilTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-ddl-iceberg.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-ddl-parquet.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
26 files changed, 333 insertions(+), 235 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I505ab11cfa1024feb4ceac4cffe9c3283be228ce
Gerrit-Change-Number: 22897
Gerrit-PatchSet: 9
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>

Reply via email to