> On June 22, 2012, 5:21 a.m., Carl Steinbach wrote: > > jdbc/src/java/org/apache/hadoop/hive/jdbc/Utils.java, line 51 > > <https://reviews.apache.org/r/5507/diff/2/?file=115803#file115803line51> > > > > Change the name to 'isEmbeddedMode'?
Done > On June 22, 2012, 5:21 a.m., Carl Steinbach wrote: > > jdbc/src/java/org/apache/hadoop/hive/jdbc/Utils.java, line 128 > > <https://reviews.apache.org/r/5507/diff/2/?file=115803#file115803line128> > > > > Space between method name and parameter list. Done > On June 22, 2012, 5:21 a.m., Carl Steinbach wrote: > > jdbc/src/test/org/apache/hadoop/hive/jdbc/TestJdbcDriver.java, line 89 > > <https://reviews.apache.org/r/5507/diff/2/?file=115804#file115804line89> > > > > Doesn't seem like the String arrays really add anything. May as well > > remove them. It makes it easier to compare the then in resultset, at least the conf settings. Removed the hiveVar[]. > On June 22, 2012, 5:21 a.m., Carl Steinbach wrote: > > jdbc/src/test/org/apache/hadoop/hive/jdbc/TestJdbcDriver.java, line 210 > > <https://reviews.apache.org/r/5507/diff/2/?file=115804#file115804line210> > > > > Might be a good idea to create a new connection handle for this test > > and set the configuration parameters for this URL only. ok. Changed the setup() to set the additional parameters only for this test. > On June 22, 2012, 5:21 a.m., Carl Steinbach wrote: > > jdbc/src/java/org/apache/hadoop/hive/jdbc/Utils.java, line 156 > > <https://reviews.apache.org/r/5507/diff/2/?file=115803#file115803line156> > > > > The apache httpcomponents library has an URLEncodedUtils.parse() method > > that does this. I'd prefer that we use it here since it reduces our test > > burden and probably does escaping. > > > > > > http://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/client/utils/URLEncodedUtils.html > > hmm ... that's going add a dependency on a new library just for that one small parsing. I have it to use a regex to make it even simpler. Please take a look and let me know if you still prefer using httpcomponents > On June 22, 2012, 5:21 a.m., Carl Steinbach wrote: > > jdbc/src/java/org/apache/hadoop/hive/jdbc/Utils.java, line 152 > > <https://reviews.apache.org/r/5507/diff/2/?file=115803#file115803line152> > > > > We may want to support syntax like this in the future: > > > > jdbc://<hostname>/<db_name>;x=1?hive.a=2 > > > > > > Where x=1 is a configuration parameter for the driver as opposed to > > something that should get set in the HiveConf. In order to be forward > > compatible with the future syntax we should make sure that dbName is the > > substring up to the first ';'. sure. In fact the dbname itself is really a parameter to driver. And the driver is currently not switching to the db specified in the URL. I will log a separate ticket and submit a patch (execute 'database <db>' after the connection is established). - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5507/#review8473 ----------------------------------------------------------- On June 22, 2012, 2:48 a.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5507/ > ----------------------------------------------------------- > > (Updated June 22, 2012, 2:48 a.m.) > > > Review request for hive, Carl Steinbach and Carl Steinbach. > > > Description > ------- > > Support passing configuration and substitution variable as part of JDBC > connection string. > The new format of the URL is > jdbc:hive://<host>:<port>/dbName?hive_conf_list#hive_var_list , where the > optional conf and var lists are semicolon separated <key>=<val> pairs. As > before, if the host/port is not specified, it the driver runs an embedded > hive. > examples - > jdbc:hive://ubuntu:11000/db2?hive.cli.conf.printheader=true;hive.exec.mode.local.auto.inputbytes.max=9999#stab=salesTable;icol=customerID > jdbc:hive://?hive.cli.conf.printheader=true;hive.exec.mode.local.auto.inputbytes.max=9999#stab=salesTable;icol=customerID > > The patch include new routines to parse the URL. The conf values are added to > HiveConf when hive is running in embedded mode otherwise they are configured > using 'set' statement. The variable substitution is only used in case of > embedded mode. > > > This addresses bug HIVE-3166. > https://issues.apache.org/jira/browse/HIVE-3166 > > > Diffs > ----- > > jdbc/src/java/org/apache/hadoop/hive/jdbc/HiveConnection.java 6618243 > jdbc/src/java/org/apache/hadoop/hive/jdbc/HiveDriver.java c61425f > jdbc/src/java/org/apache/hadoop/hive/jdbc/Utils.java 24d5882 > jdbc/src/test/org/apache/hadoop/hive/jdbc/TestJdbcDriver.java f6a904f > > Diff: https://reviews.apache.org/r/5507/diff/ > > > Testing > ------- > > Ran JDBC tests. > Added new test case for the extended URL format. > > > Thanks, > > Prasad Mujumdar > >
