You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2024/12/07 00:12:09 UTC

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22184

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................

IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests

This patch enable VALIDATE_CARDINALITY test options in several planner
tests that touch aggregation node. Enabling it has reveal three bugs.

First, in IMPALA-13480, cardinality estimate of MERGE phase aggregation
is not capped against the output cardinality of the EXCHANGE node. This
patch fix it by adding such capping.

Second, tuple-based optimization IMPALA-13480 can cause cardinality
underestimation if HAVING predicate exist. This is due to the default
selectivity of 10% applied for each HAVING predicate. This patch skip
tuple-based optimization if AggregationNode.conjuncts_ is ever not
empty. It will stay skipped on stats recompute, even if conjuncts_ is
transfered into the next Merge AggregationNode above the plan. The
optimization skip causes following PlannerTest (under
testdata/workloads/functional-planner/queries/PlannerTest/) to revert
their cardinality estimation to their state pior to IMPALA-13405:
- tpcds/tpcds-q39a.test
- tpcds/tpcds-q39b.test
- tpcds_cpu_cost/tpcds-q39a.test
- tpcds_cpu_cost/tpcds-q39b.test
In the future, we should consider raising the default selectivity for
HAVING predicate and undo this skipping logic (IMPALA-13542).

Third, is missing stats recompute after conjunct transfer in multi-phase
aggregation. This will be fixed separately by IMPALA-13526.

Testing:
- Enable cardinality validation in testMultipleDistinct*
- Update aggregation.test to reflect current PlannerTest output.
Added some test cases in aggregation.test.
- Run and pass TpcdsPlannerTest and TpcdsCpuPlannerTest.
- Selectively run some more planner tests that touch AggregationNode and
pass them.

Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
---
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/multiple-distinct-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/multiple-distinct-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds/tpcds-q39a.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds/tpcds-q39b.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-q39a.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-q39b.test
9 files changed, 1,323 insertions(+), 456 deletions(-)

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 20:26:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 14:54:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 14:59:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 3:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Dec 2024 15:36:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................

IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests

This patch enable VALIDATE_CARDINALITY test options in several planner
tests that touch aggregation node. Enabling it has revealed three bugs.

First, in IMPALA-13405, cardinality estimate of MERGE phase aggregation
is not capped against the output cardinality of the EXCHANGE node. This
patch fix it by adding such capping.

Second, tuple-based optimization IMPALA-13405 can cause cardinality
underestimation if HAVING predicate exist. This is due to the default
selectivity of 10% applied for each HAVING predicate. This patch skip
tuple-based optimization if AggregationNode.conjuncts_ is ever not
empty. It will stay skipped on stats recompute, even if conjuncts_ is
transfered into the next Merge AggregationNode above the plan. The
optimization skip causes following PlannerTest (under
testdata/workloads/functional-planner/queries/PlannerTest/) to revert
their cardinality estimation to their state pior to IMPALA-13405:
- tpcds/tpcds-q39a.test
- tpcds/tpcds-q39b.test
- tpcds_cpu_cost/tpcds-q39a.test
- tpcds_cpu_cost/tpcds-q39b.test
In the future, we should consider raising the default selectivity for
HAVING predicate and undo this skipping logic (IMPALA-13542).

Third, is missing stats recompute after conjunct transfer in multi-phase
aggregation. This will be fixed separately by IMPALA-13526.

Testing:
- Enable cardinality validation in testMultipleDistinct*
- Update aggregation.test to reflect current PlannerTest output.
  Added some test cases in aggregation.test.
- Run and pass TpcdsPlannerTest and TpcdsCpuPlannerTest.
- Selectively run some more planner tests that touch AggregationNode and
  pass them.

Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Reviewed-on: http://gerrit.cloudera.org:8080/22184
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/multiple-distinct-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/multiple-distinct-predicates.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds/tpcds-q39a.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds/tpcds-q39b.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-q39a.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds_cpu_cost/tpcds-q39b.test
9 files changed, 1,324 insertions(+), 457 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17629/ : 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/22184
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 07 Dec 2024 00:39:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG@10
PS2, Line 10: reveal
> nit: revealed
Done


http://gerrit.cloudera.org:8080/#/c/22184/2/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:

http://gerrit.cloudera.org:8080/#/c/22184/2/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@349
PS2, Line 349: 
> nit: remove braces, as the line is not broken?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Dec 2024 19:09:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.

Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG@10
PS2, Line 10: tests that touch aggregation node. Enabling it has reveal three bugs.
This commit identifies actual bugs that you're also fixing. I'd like the ticket description to reflect that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2024 19:01:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG@10
PS2, Line 10: tests that touch aggregation node. Enabling it has reveal three bugs.
> This commit identifies actual bugs that you're also fixing. I'd like the ti
I added JIRA comment descibing the bug:
https://issues.apache.org/jira/browse/IMPALA-13480?focusedCommentId=17904272&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17904272



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2024 20:34:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Michael Smith, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................

IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests

This patch enable VALIDATE_CARDINALITY test options in several planner
tests that touch aggregation node. Enabling it has reveal three bugs.

First, in IMPALA-13405, cardinality estimate of MERGE phase aggregation
is not capped against the output cardinality of the EXCHANGE node. This
patch fix it by adding such capping.

Second, tuple-based optimization IMPALA-13405 can cause cardinality
underestimation if HAVING predicate exist. This is due to the default
selectivity of 10% applied for each HAVING predicate. This patch skip
tuple-based optimization if AggregationNode.conjuncts_ is ever not
empty. It will stay skipped on stats recompute, even if conjuncts_ is
transfered into the next Merge AggregationNode above the plan. The
optimization skip causes following PlannerTest (under
testdata/workloads/functional-planner/queries/PlannerTest/) to revert
their cardinality estimation to their state pior to IMPALA-13405:
- tpcds/tpcds-q39a.test
- tpcds/tpcds-q39b.test
- tpcds_cpu_cost/tpcds-q39a.test
- tpcds_cpu_cost/tpcds-q39b.test
In the future, we should consider raising the default selectivity for
HAVING predicate and undo this skipping logic (IMPALA-13542).

Third, is missing stats recompute after conjunct transfer in multi-phase
aggregation. This will be fixed separately by IMPALA-13526.

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 2: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG@10
PS2, Line 10: reveal
nit: revealed


http://gerrit.cloudera.org:8080/#/c/22184/2/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:

http://gerrit.cloudera.org:8080/#/c/22184/2/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@349
PS2, Line 349: }
nit: remove braces, as the line is not broken?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Dec 2024 18:52:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22184/2//COMMIT_MSG@40
PS2, Line 40: - Selectively run some more planner tests that touch AggregationNode and
            :   pass them.
This is the list of test that I run.

cd $IMPALA_HOME/fe
mvn -B -fae test -Dtest="PlannerTest#testAggregation
+testPartitionKeyScansDefault
+testPreaggBytesLimit
+testProcessingCost
+testProcessingCostPlanAdmissionSlots
+testSpillableBufferSizing
+testAggCardinality
+testAnalyticFns
+testBloomFilterAssignment
+testIcebergV2TableScans
+testIcebergV2TableScansHashJoin
+testIcebergV2TableScansWithResources
+testJoinOrder
+testJoinsWithHDFSNumRowsEstDisabled
+testOuterJoins
+testMaxRowSize
+testMultipleDistinct
+testMultipleDistinctLimit
+testMultipleDistinctMaterialization
+testMultipleDistinctPredicates
+testResourceRequirements
+testShuffleByDistinctExprs
+testSmallQueryOptimization
,CardinalityTest.testAggregationNodeGroupByCardinalityCapping"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 07 Dec 2024 00:18:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 14:59:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.

Hello Michael Smith, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................

IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests

This patch enable VALIDATE_CARDINALITY test options in several planner
tests that touch aggregation node. Enabling it has revealed three bugs.

First, in IMPALA-13405, cardinality estimate of MERGE phase aggregation
is not capped against the output cardinality of the EXCHANGE node. This
patch fix it by adding such capping.

Third, is missing stats recompute after conjunct transfer in multi-phase
aggregation. This will be fixed separately by IMPALA-13526.

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13480: VALIDATE CARDINALITY in some aggregation tests

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22184 )

Change subject: IMPALA-13480: VALIDATE_CARDINALITY in some aggregation tests
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17660/ : 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/22184
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iadb4af9fd65fdb85b66fae1e403ccec8ca5eb102
Gerrit-Change-Number: 22184
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Dec 2024 19:34:59 +0000
Gerrit-HasComments: No