Alex Behm has posted comments on this change.

Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to 
HMS.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8112/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

Line 800:       
partition.putToParameters(MetastoreShim.statsGeneratedViaStatsTaskParam());
> Are partition level stats also affected, just wanted to make sure that we d
I checked the Hive/HMS code carefully and partition-level stat are not 
affected. Feel free to look yourself. The problematic stats-wiping function is 
MetastoreUtils.updateTableStatsFast().

While at some level I'd love to pass DO_NOT_UPDATE_STATS in all RPC to the HMS, 
I feel like being too defensive is confusing to readers. Adding the flag here 
will actually add the table property to the partition which is also weird.


PS1, Line 822: Pair<String, String> statsTaskParam = 
MetastoreShim.statsGeneratedViaStatsTaskParam();
             :     msTbl.putToParameters(statsTaskParam.first, 
statsTaskParam.second);
> This looks redundant and is overwritten anyway right?
Setting this flag is required for for stats changes to take effect (table or 
partition). Why do you way this is redundant? It is different than the 
DO_NOT_UPDATE_STATS flag.

Added comments.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-HasComments: Yes

Reply via email to