impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vuk Ercegovac (Code Review)" <>
Date Wed, 27 Sep 2017 06:54:45 GMT
Vuk Ercegovac has posted comments on this change. ( )


Patch Set 1:

File fe/src/main/java/org/apache/impala/analysis/
PS1, Line 55: >)]
I was expecting to see a mention of REPEATABLE here (per change description).
PS1, Line 58: partition-level row counts are not modified.
does this allow for the state of table vs. partition stats to be inconsistent (e.g., combining
per-partition stats may not add up to the table level stats)?
File fe/src/main/java/org/apache/impala/planner/
PS1, Line 161: .
pls clarify if these should never be valid at the same time.
PS1, Line 184: partition
nit: partitions
PS1, Line 780: cardinality_
it would be clearer if this is named outputCardinality_ (the comment for this method refers
to it this way, so there's already some drift).
PS1, Line 830: getNumRows
used at 4 call sites in this for-loop. assign it to a variable.
will also make things more consistent with the non-partitioned case at L845.
PS1, Line 1122: tbl_.getNumClusteringCols() > 0
this is used in several places-- it would be clearer if it was wrapped in something like "isPartitioned()"
File fe/src/main/java/org/apache/impala/service/
PS1, Line 744: Impala
got confused here since previous line mentions Hive and Impala tables (why should the partitions
be Impala partitions in all cases?)
PS1, Line 795: Hive table
given previous comment referencing Hive and Impala tables, I'm unclear about what is specific
to Hive tables here.
PS1, Line 860: totalFileBytes / sampleFileBytes
is returned result guaranteed to be >= val?

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f3e72471ac563adada4a4156033a85852b7c8b7
Gerrit-Change-Number: 8136
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-Comment-Date: Wed, 27 Sep 2017 06:54:45 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message