----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16910/#review31892 -----------------------------------------------------------
The patch itself look fine. Though I think it's not covering the case when you already have some hiveconf or hivevars in the URL along with the command line options. Looks like we can support only one way of adding these parameters at a time. If there's a mix and match, it would be better to catch that and raise an error ? beeline/src/java/org/apache/hive/beeline/DatabaseConnection.java <https://reviews.apache.org/r/16910/#comment60657> Nit: whitespace - Prasad Mujumdar On Jan. 15, 2014, 5:43 p.m., Xuefu Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16910/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2014, 5:43 p.m.) > > > Review request for hive. > > > Bugs: HIVE-6173 > https://issues.apache.org/jira/browse/HIVE-6173 > > > Repository: hive-git > > > Description > ------- > > Introduced --hiveconf option on beeline option. > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java c5e36a5 > beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 04802bc > beeline/src/java/org/apache/hive/beeline/DatabaseConnection.java 553722d > beeline/src/main/resources/BeeLine.properties 408286d > > itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java > 539ebdb > > Diff: https://reviews.apache.org/r/16910/diff/ > > > Testing > ------- > > Unit test added. > > > Thanks, > > Xuefu Zhang > >
