Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-1683: Allow REFRESH on a single partition ......................................................................
Patch Set 7: (10 comments) http://gerrit.cloudera.org:8080/#/c/3385/6/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: PS6, Line 1182: hdfspartition.getPartitionName(); > this method was already there in the hdfs table class. the reason i think t Sorry didn't realize there was another one with this input param. That's fine, let's leave it as is. http://gerrit.cloudera.org:8080/#/c/3385/7/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: PS7, Line 1172: HdfsTable hdfsTable = ((HdfsTable) tbl); Move above L1176 PS7, Line 1173: Db db = tbl.getDb(); : TTableName tblName = new TTableName(tbl.getDb().getName().toLowerCase(), : tbl.getName().toLowerCase()); Move above L1187 PS7, Line 1189: refreshedPartition Why refreshed? This is just the HMS partition object. You can simply call it hmsPartition. http://gerrit.cloudera.org:8080/#/c/3385/7/fe/src/main/java/com/cloudera/impala/catalog/HdfsTable.java File fe/src/main/java/com/cloudera/impala/catalog/HdfsTable.java: PS7, Line 1926: Returns the name of the partition described by the input thrift : // partition spec object How about "Constructs a partition name from a list of TPartitionKeyValue objects." PS7, Line 1945: of a partition by removing the old : // metadata using the 'oldPartition' object and adding back fresh metadata to : // the table using the newly fetched 'refershedPartition' object How about "of partition 'oldPartition' by removing it from the table and reconstructing it from the HMS partition object 'hmsPartition'". PS7, Line 1948: refreshPartition Maybe rename to 'reloadPartition'. PS7, Line 1949: refershedPartition Can you change this to hmsPartition? This is not a refreshed HdfsTable::HdfsPartition object that you simply use to replace oldPartition. It is an HMS partition object based on which you reconstruct the HdfsPartition object that replaces oldPartition. http://gerrit.cloudera.org:8080/#/c/3385/7/fe/src/test/java/com/cloudera/impala/analysis/AnalyzerTest.java File fe/src/test/java/com/cloudera/impala/analysis/AnalyzerTest.java: PS7, Line 659: AnalysisError("refresh functional.alltypessmall partition (year=2009, int_col=10)", Can you also add the cases: a) the same partition column shows up multiple times e.g. refresh functional.alltypessmall partition (year = 2009, year=2010); b) no partition columns are used c) the partition key value is not compatible with the partition column e.g. year = "dimitris" d) partition key value is NULL, e.g. year = NULL PS7, Line 664: remove extra space -- To view, visit http://gerrit.cloudera.org:8080/3385 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bf5dd630f2e175be5800187bbf9337c91f82bd8 Gerrit-PatchSet: 7 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-HasComments: Yes
