Return-Path: X-Original-To: apmail-hive-dev-archive@www.apache.org Delivered-To: apmail-hive-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7211418C9B for ; Thu, 25 Jun 2015 21:54:38 +0000 (UTC) Received: (qmail 19310 invoked by uid 500); 25 Jun 2015 21:54:37 -0000 Delivered-To: apmail-hive-dev-archive@hive.apache.org Received: (qmail 19234 invoked by uid 500); 25 Jun 2015 21:54:37 -0000 Mailing-List: contact dev-help@hive.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hive.apache.org Delivered-To: mailing list dev@hive.apache.org Received: (qmail 19218 invoked by uid 99); 25 Jun 2015 21:54:37 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 25 Jun 2015 21:54:37 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 79DB4AAE6E; Thu, 25 Jun 2015 21:54:36 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============8455780507892561655==" MIME-Version: 1.0 Subject: Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command From: "Xuefu Zhang" To: chinnarao@huawei.com, "Xuefu Zhang" Cc: "cheng xu" , "hive" Date: Thu, 25 Jun 2015 21:54:36 -0000 Message-ID: <20150625215436.3113.12469@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Xuefu Zhang" X-ReviewGroup: hive X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/35107/ X-Sender: "Xuefu Zhang" References: <20150625055406.3113.2635@reviews.apache.org> In-Reply-To: <20150625055406.3113.2635@reviews.apache.org> Reply-To: "Xuefu Zhang" X-ReviewRequest-Repository: hive-git --===============8455780507892561655== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review89421 ----------------------------------------------------------- beeline/src/java/org/apache/hive/beeline/Commands.java (line 721) Nit: Keep @Override in a separate line. beeline/src/java/org/apache/hive/beeline/Commands.java (line 722) I think getHiveVariables() is in the same class, which can be directly accessed. beeline/src/java/org/apache/hive/beeline/Commands.java (line 760) I think getHiveVariables() can be private. beeline/src/java/org/apache/hive/beeline/Commands.java (line 772) getHiveConf() can be proviate. Remove the caller from BeeLine.java. beeline/src/java/org/apache/hive/beeline/Commands.java (line 850) This used to call BeeLine.executeFile() now you have a new implementation. I don't quite follow why. Regardless, I see two problems. 1. we should remove the 2nd argument as all callers provide "false". 2. Some part of the code in executeFile() is to fix some problem with the last line in the script file. Will your new implementation catches that? 3. It's my understanding that in Hive CLI compatible mode, commands in source file should follow Hive CLI syntax, while in normal mode, it should follow Beeline syntax. Is this what's done here? - Xuefu Zhang On June 25, 2015, 5:54 a.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35107/ > ----------------------------------------------------------- > > (Updated June 25, 2015, 5:54 a.m.) > > > Review request for hive, chinna and Xuefu Zhang. > > > Bugs: HIVE-6791 > https://issues.apache.org/jira/browse/HIVE-6791 > > > Repository: hive-git > > > Description > ------- > > Summary: > 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) > 2) support substitution for source command > 3) add some unit test for substitution > 4) add one way to get the configuration from HS2 > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e > beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 > beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c > common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION > common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION > common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f > ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 > ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 > ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 > ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 > ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d > ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b > > Diff: https://reviews.apache.org/r/35107/diff/ > > > Testing > ------- > > Unit test passed > > > Thanks, > > cheng xu > > --===============8455780507892561655==--