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 <jes...@gmail.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 01 Aug 2018 23:26:11 +0000 Gerrit-HasComments: Yes
