Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19354 )

Change subject: IMPALA-11787, IMPALA-11516: Cardinality estimate for UNION in 
Iceberg position-delete plans can double the actual table cardinality
......................................................................


Patch Set 2:

(7 comments)

Thanks for the comments!

I improved the non-identity conjuncts part in IcebergScanPlanner, to deal with 
more complex conjuncts. Also added tests for it.

http://gerrit.cloudera.org:8080/#/c/19354/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19354/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-11787,
> Maybe we could tag IMPALA-11516 too
Done


http://gerrit.cloudera.org:8080/#/c/19354/1/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/19354/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1141
PS1, Line 1141:     if (sampleParams_ != null) {
> line has trailing whitespace
Done


http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java:

http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@105
PS1, Line 105: cardinalities
> nit: singular
It computes input and output cardinalities, similarly to 
HdfsScanNode.computeCardinalities().


http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@110
PS1, Line 110:    */
> Nit: delete line
Done


http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@129
PS1, Line 129:       if (t.isCollectionType()) cardinality_ *= 
PlannerContext.AVG_COLLECTION_SIZE;
> Can cardinality_ be -1? In HdfsScanNode it can be but here maybe it cannot.
Done


http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java
File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java:

http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@196
PS1, Line 196:     IcebergScanNode dataScanNode = new IcebergScanNode(
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/19354/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@227
PS1, Line 227:     IcebergScanNode dataScanNode = new IcebergScanNode(
> nit: indentation
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2927c58c4adfd0ba1e135b63454ac9b07991cbf
Gerrit-Change-Number: 19354
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tamas Mate <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 14 Dec 2022 10:56:22 +0000
Gerrit-HasComments: Yes

Reply via email to