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 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@7 PS2, Line 7: IMPALA-6994:Avoid reloading a table's HMS data for file-only operations nit: add a space http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@9 PS2, Line 9: remove the description block's indent http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@10 PS2, Line 10: unecessary nit: check spelling and remove extra spaces. http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@9 PS2, Line 9: unpartitioned HDFS table, : or to an existing partition simplify to just HDFS table, or is there some case that's being excluded? http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@14 PS2, Line 14: This extra call to HMS introduces a point of failure, we need to handle : for error scenario when Hive MetaStore crashes So is this change an optimization, a correctness/robustness fix, or both? Also, I don't understand the point about a failure-- pls add an example with a sequence of steps that currently leads to an error. Can add such an example to the jira. http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@15 PS2, Line 15: This change removes the : extra call to Hive Meta Store for the case when a row is inserted to an : existing partition in HDFS table or when a row is added to unpartitioned : table. Thus, an optimization as it reduces the call to Hive Meta Store : during Update of Catalog. lots of redundancy here with the prev. paragraph-- pls condense or remove. http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@21 PS2, Line 21: Testing: Ran core test without failure. Is there any new scenario that you want to test for this change? If existing tests cover it, is there any way to assert that this change is operating as expected? http://gerrit.cloudera.org:8080/#/c/10587/2/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/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1252 PS2, Line 1252: nonewPartitionHint needs a comment. -- 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: 2 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 06 Jul 2018 16:19:48 +0000 Gerrit-HasComments: Yes
