----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5381/#review8331 -----------------------------------------------------------
LICENSE <https://reviews.apache.org/r/5381/#comment17949> This is the text posted on the SQLLine site site from where the hive build is downloading the jar. Since we are not using the source jar or making source changes, do we need to look at any other license terms than one listed for distribution ? NOTICE <https://reviews.apache.org/r/5381/#comment17950> Done bin/ext/sqlline.sh <https://reviews.apache.org/r/5381/#comment17952> hmm .. how about make that under a specific option like '--local or -l'. If an embedded mode is needed, then its better be invoked explicitly than overloading the host (or absence of it). I will log a separate ticket or subtask to address that if you agree. bin/ext/sqlline.sh <https://reviews.apache.org/r/5381/#comment17953> was testing this earlier with a dummy HS2 server with default port 10001. sorry about that. Fixed bin/ext/sqlline.sh <https://reviews.apache.org/r/5381/#comment17957> removed the user/passwd arguments. Its not actually mandatory for sqlline tool invocation (if the URL is specified) and hive server currently doesn't require it. The 'silent' options suppresses version information that sqlline prints, From what I understand the JDBC driver doesn't provide very meaningful version details. It can also clutter the output and create problem for test output comparisons due to version information. For command line invocation, silent is default false and --silent makes it true. Changed that to --silent=true for better readability. bin/ext/sqlline.sh <https://reviews.apache.org/r/5381/#comment17959> yes, the SQLLine invokes JDBC methods to check autoCommit, isolation and extraNameCharacters which are not supported by hive. bin/ext/sqlline.sh <https://reviews.apache.org/r/5381/#comment17961> The options can be changed within the editor after it starts. bin/ext/util/execHiveCmd.sh <https://reviews.apache.org/r/5381/#comment17963> Done bin/ext/util/execHiveCmd.sh <https://reviews.apache.org/r/5381/#comment17964> Done bin/ext/util/execHiveCmd.sh <https://reviews.apache.org/r/5381/#comment17968> ok. I was planing to add a java wrapper, primarily for the commandline or batch execution of HQL. The SQLLine currently requires a script that's written using its own command syntax and doesn't support equivalent of '-e HQL' or '-f <FILE.QL>' I will log a ticket to create a wrapper that address all these three issues. ivy.xml <https://reviews.apache.org/r/5381/#comment17951> right. Didn't remote this in the previous patch. Done ivy/libraries.properties <https://reviews.apache.org/r/5381/#comment17972> The directory and file names for version 1.0.2 are following different conventions that makes it hard to access it in ivy. I will followup with SQLLine community to address it if possible. - Prasad Mujumdar On June 18, 2012, 5:59 a.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5381/ > ----------------------------------------------------------- > > (Updated June 18, 2012, 5:59 a.m.) > > > Review request for hive and Carl Steinbach. > > > Description > ------- > > The patch is to include command line SQL editor SQLLine into Hive > distribution. The tool can be invoked using 'hive --service sqlline <host> > [port]'. It requires the HiveServer running on the given host/port. > > The ivy dependencies are updated to include sqlline. The hive scripts are > updated to executing SQLLine with the required connection URL and other > command line options. The LICENSE and NOTICE files are updated to include > SQLLine information. > > > Diffs > ----- > > LICENSE 05085da > NOTICE 871fdde > bin/ext/sqlline.sh PRE-CREATION > bin/ext/util/execHiveCmd.sh 167cc40 > cli/ivy.xml ab949b1 > ivy.xml e83437e > ivy/ivysettings.xml fb6f4b8 > ivy/libraries.properties 8461da1 > > Diff: https://reviews.apache.org/r/5381/diff/ > > > Testing > ------- > > > Thanks, > > Prasad Mujumdar > >
