[Impala-ASF-CR] IMPALA-6994: Avoid reloading a table's HMS data for file-only operations

["Vuk Ercegovac (Code Review)" <ge...@cloudera.org>]

Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10587 )

Change subject: IMPALA-6994: Avoid reloading a table's HMS data for file-only operations
......................................................................


Patch Set 4:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/10587/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10587/4//COMMIT_MSG@9
PS4, Line 9: new row
this should apply more generally to new files, not just rows? either way, the wording here is inconsistent with the title, which refers to "file-only operations".


http://gerrit.cloudera.org:8080/#/c/10587/4//COMMIT_MSG@17
PS4, Line 17: the
all? which partitions?


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1417
PS4, Line 1417: a
nit: an


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1420
PS4, Line 1420: by calling
nit: from


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

http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@254
PS4, Line 254: reloads the table schema from Hive Meta Store
can in-line this comment directly on L259. same with the other descriptions of what each flag does when set. 

doing so will remove some redundancy in the comment.


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@585
PS4, Line 585: noneOf(MetaDataLoadFlag.class);
             :         flags.add
simplify to .of(...) since we're always adding RELOAD_PARTITION_METADATA.


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@663
PS4, Line 663: MetaDataLoadFlag of type 'RELOAD_TABLE_METADATA', 'RELOAD_PARTITION_METADATA',
             :    * 'RELOAD_FILE_METADATA'
replace with just flags.


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@670
PS4, Line 670: msTbl
why is this pulled out now as a parameter. I see one place where its used-- what requires this change?
if its really needed, please add a comment (e.g., what does a null msTbl do?)


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3484
PS4, Line 3484: noneOf(MetaDataLoadFlag.class);
              :       flags.add(MetaDataLoadFlag.RELOAD_FILE_METADATA)
simplify to of(...) since the flag on L3485 is always included.


http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3486
PS4, Line 3486:  == true
remove



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I331bb0371fde287f43a85b025b4f98cb45f3eb3c
Gerrit-Change-Number: 10587
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Balazs Jeszenszky <je...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Aug 2018 23:26:11 +0000
Gerrit-HasComments: Yes