----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5476/#review8529 -----------------------------------------------------------
Code changes look good. Can you please add a q-file testcase? Directions are located here: https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-AddaUnitTest - Carl Steinbach On June 25, 2012, 3:38 a.m., Shengsheng Huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5476/ > ----------------------------------------------------------- > > (Updated June 25, 2012, 3:38 a.m.) > > > Review request for hive, Carl Steinbach and ransom. > > > Description > ------- > > The patch passed the test case provided in the bug. It feed partition p=2 > with correct location. > > The bug came from the wrong node in the AST, shown as below: > (TOK_ALTERTABLE_ADDPARTS t (TOK_PARTSPEC (TOK_PARTVAL p 1)) > (TOK_PARTITIONLOCATION '/Users/carl/1') (TOK_PARTSPEC (TOK_PARTVAL p 2)) > (TOK_PARTITIONLOCATION '/Users/carl/1')) > > In above AST, TOK_PARTSPEC (specification of partition) and > TOK_PARTITIONLOCATION (partition location) nodes are all siblings, which does > not make much sense to me. In my patch I rearranged the AST a bit by > attaching the TOK_PARTITIONLOCATION to TOK_PARTSPEC node and it fixed the > problem. The new AST is shown below. DDLSemanticAnalyzer is changed > accordingly. > > (TOK_ALTERTABLE_ADDPARTS t (TOK_PARTSPEC (TOK_PARTVAL p 1) > (TOK_PARTITIONLOCATION '/Users/carl/1')) (TOK_PARTSPEC (TOK_PARTVAL p 2) > (TOK_PARTITIONLOCATION '/Users/carl/2'))) > > > This addresses bug HIVE-3148. > https://issues.apache.org/jira/browse/HIVE-3148 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 1352422 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g > 1352422 > > Diff: https://reviews.apache.org/r/5476/diff/ > > > Testing > ------- > > > Thanks, > > Shengsheng Huang > >
