You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2025/07/17 20:05:55 UTC
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/23185
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 224 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/23185/1
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19958/ : 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/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jul 2025 19:47:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode
when the statistics are missing because they were not computed with
the compute statistics command.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Reviewed-on: http://gerrit.cloudera.org:8080/23185
Reviewed-by: Aman Sinha <am...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 247 insertions(+), 176 deletions(-)
Approvals:
Aman Sinha: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Anonymous Coward (816), Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23185
to look at the new patch set (#5).
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode
when the statistics are missing because they were not computed with
the compute statistics command.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 247 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/23185/5
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java
File fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java:
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java@75
PS3, Line 75: public HdfsEstimatedTableStats() {
> Since all statistics are 'estimated' (well, row counts are more accurate th
Correction : I meant HdfsEstimatedMissingTableStats (added the 'd'in the estimated)
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sun, 20 Jul 2025 15:44:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19968/ : 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/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Sun, 27 Jul 2025 03:19:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19903/ : 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/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 17 Jul 2025 20:35:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/23185/4/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
File fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java:
http://gerrit.cloudera.org:8080/#/c/23185/4/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java@40
PS4, Line 40: public final int numPartitionsWithNumRows_;
nit: preserving the code comments here could be nice if they're still relevant.
http://gerrit.cloudera.org:8080/#/c/23185/4/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java@108
PS4, Line 108: partitionNumRows_ = partitionNumRows;
Why do it this way? So the class variables can be final?
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jul 2025 21:17:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/23185/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23185/3//COMMIT_MSG@10
PS3, Line 10: statistics for the original planner are estimated within HdfsScanNode
> nit: to be clear, the estimation here is done when there's missing stats (s
Done
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java
File fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java:
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java@75
PS3, Line 75:
> Since all statistics are 'estimated' (well, row counts are more accurate th
Done
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java@75
PS3, Line 75:
> Correction : I meant HdfsEstimatedMissingTableStats (added the 'd'in the es
Done
http://gerrit.cloudera.org:8080/#/c/23185/3/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/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@341
PS3, Line 341: eStats estima
> Similar to the class naming, this could be changed to estimatedMissingStats
Done
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Jul 2025 22:09:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/23185/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23185/3//COMMIT_MSG@10
PS3, Line 10: statistics for the original planner are estimated within HdfsScanNode.
nit: to be clear, the estimation here is done when there's missing stats (since stats are always 'estimated' in any case). The commit message could be updated to indicate that.
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java
File fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java:
http://gerrit.cloudera.org:8080/#/c/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java@75
PS3, Line 75: public HdfsEstimatedTableStats() {
Since all statistics are 'estimated' (well, row counts are more accurate than a column stat such as NDV but row counts correspond to the last time stats were collected, so they are still estimates), I think the naming here is not specific enough. This class should be used only when there's missing stats. How about HdfsEstimateMissingTableStats. A bit verbose but distinguishes from the case where normal estimated stats are available.
http://gerrit.cloudera.org:8080/#/c/23185/3/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/23185/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@341
PS3, Line 341: plannerStats_
Similar to the class naming, this could be changed to estimatedMissingStats_ or something similar with 'missing' in it.
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sun, 20 Jul 2025 15:26:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Anonymous Coward (816), Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23185
to look at the new patch set (#6).
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode
when the statistics are missing because they were not computed with
the compute statistics command.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 247 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/23185/6
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 6
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Jul 2025 06:59:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19904/ : 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/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 18 Jul 2025 00:15:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Anonymous Coward (816), Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23185
to look at the new patch set (#4).
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode
when the statistics are missing because they were not computed with
the compute statistics command.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 224 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/23185/4
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 7: Code-Review+2
(1 comment)
LGTM. Carry forward Michael's +1 and bumping mine to +2
http://gerrit.cloudera.org:8080/#/c/23185/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/23185/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@243
PS5, Line 243: estimatedMissingStats_.partitio
> This part of the comment is incorrect since this is supposed to reference t
Done
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Sun, 27 Jul 2025 22:11:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19932/ : 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/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Jul 2025 21:40:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19952/ : 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/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jul 2025 17:56:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/23185/4/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
File fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java:
http://gerrit.cloudera.org:8080/#/c/23185/4/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java@40
PS4, Line 40: public final int numPartitionsWithNumRows_;
> nit: preserving the code comments here could be nice if they're still relev
Done
http://gerrit.cloudera.org:8080/#/c/23185/4/fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java@108
PS4, Line 108: partitionNumRows_ = partitionNumRows;
> Why do it this way? So the class variables can be final?
Yeah, that was pretty much it. I have a preference towards final variables (and then there's also no real need for a "getter" method either.)
If you have a different way that you think is more elegant, I'm open to suggestions.
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jul 2025 17:29:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Anonymous Coward (816), Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23185
to look at the new patch set (#3).
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 222 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/23185/3
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Anonymous Coward (816), Fang-Yu Rao, Riza Suminto, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23185
to look at the new patch set (#7).
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
IMPALA-14094: Prepend to commit, refactor estimated table stats
IMPALA-14094 adds statistics for the Calcite planner. The row count
statistics for the original planner are estimated within HdfsScanNode
when the statistics are missing because they were not computed with
the compute statistics command.
This commit refactors this estimation code so that it is shareable.
Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
---
A fe/src/main/java/org/apache/impala/planner/HdfsEstimatedMissingTableStats.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
2 files changed, 247 insertions(+), 176 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/23185/7
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12238/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Mon, 28 Jul 2025 00:46:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-14094: Prepend to commit, refactor estimated table stats
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23185 )
Change subject: IMPALA-14094: Prepend to commit, refactor estimated table stats
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/23185/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/23185/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@243
PS5, Line 243: statsNumRows_.partitionNumRows_
This part of the comment is incorrect since this is supposed to reference the estimatedMissingStats_ object.
--
To view, visit http://gerrit.cloudera.org:8080/23185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I522e5105867fa1c85df5c04a4bc6cdd5d63443f0
Gerrit-Change-Number: 23185
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jul 2025 01:49:28 +0000
Gerrit-HasComments: Yes