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 41E5A1059C for ; Wed, 18 Dec 2013 00:59:57 +0000 (UTC) Received: (qmail 13753 invoked by uid 500); 18 Dec 2013 00:59:55 -0000 Delivered-To: apmail-hive-dev-archive@hive.apache.org Received: (qmail 13618 invoked by uid 500); 18 Dec 2013 00:59:55 -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 13598 invoked by uid 99); 18 Dec 2013 00:59:55 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 18 Dec 2013 00:59:55 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 971371D3F66; Wed, 18 Dec 2013 00:59:54 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2117881694174415293==" MIME-Version: 1.0 Subject: Re: Review Request 16299: HIVE-6013: Supporting Quoted Identifiers in Column Names From: "Ashutosh Chauhan" To: "Alan Gates" , "Ashutosh Chauhan" Cc: "Harish Butani" , "hive" Date: Wed, 18 Dec 2013 00:59:54 -0000 Message-ID: <20131218005954.20506.11561@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Ashutosh Chauhan" X-ReviewGroup: hive X-ReviewRequest-URL: https://reviews.apache.org/r/16299/ X-Sender: "Ashutosh Chauhan" References: <20131216222241.26374.96643@reviews.apache.org> In-Reply-To: <20131216222241.26374.96643@reviews.apache.org> Reply-To: "Ashutosh Chauhan" X-ReviewRequest-Repository: hive-git --===============2117881694174415293== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16299/#review30570 ----------------------------------------------------------- common/src/java/org/apache/hadoop/hive/conf/HiveConf.java class PatternValidator was recently introduced in HiveConf, which doesn't let user to specify invalid value for a config key. Using that here will be useful. metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java Shall we remove this if() altogether and thus also above newly introduced method? ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java conf should be null here. If it is null, then its a bug. Also, returning null in those cases seems incorrect. Lets remove this null conf check. ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java Since this method always return true, no need for this if block. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g There can never be the case that hiveconf == null. That will be a bug. Lets remove this null check. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g It will be good to document where all Identifier is used. Can be lifted straight from your html document. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g Good to add a note here saying QuotedIdentifier only optionally available for columns as of now. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g Not related for this patch, but if you feel like it, ll be good to add comment about where CharSetNames are used. Not necessary though. - Ashutosh Chauhan On Dec. 16, 2013, 10:22 p.m., Harish Butani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16299/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2013, 10:22 p.m.) > > > Review request for hive, Ashutosh Chauhan and Alan Gates. > > > Bugs: HIVE-6013 > https://issues.apache.org/jira/browse/HIVE-6013 > > > Repository: hive-git > > > Description > ------- > > Hive's current behavior on Quoted Identifiers is different from the normal interpretation. Quoted Identifier (using backticks) has a special interpretation for Select expressions(as Regular Expressions). Have documented current behavior and proposed a solution in attached doc. > Summary of solution is: > Introduce 'standard' quoted identifiers for columns only. > At the langauage level this is turned on by a flag. > At the metadata level we relax the constraint on column names. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048 > itests/qtest/pom.xml 8c249a0 > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 3deed45 > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java eb26e7f > ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 321759b > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 17e6aad > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g ed9917d > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java 1e6826f > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d18ea03 > ql/src/java/org/apache/hadoop/hive/ql/parse/UnparseTranslator.java 8fe2262 > ql/src/test/queries/clientnegative/invalid_columns.q f8be8c8 > ql/src/test/queries/clientpositive/quotedid_alter.q PRE-CREATION > ql/src/test/queries/clientpositive/quotedid_basic.q PRE-CREATION > ql/src/test/queries/clientpositive/quotedid_partition.q PRE-CREATION > ql/src/test/queries/clientpositive/quotedid_skew.q PRE-CREATION > ql/src/test/queries/clientpositive/quotedid_smb.q PRE-CREATION > ql/src/test/queries/clientpositive/quotedid_tblproperty.q PRE-CREATION > ql/src/test/results/clientnegative/invalid_columns.q.out 3311b0a > ql/src/test/results/clientpositive/quotedid_alter.q.out PRE-CREATION > ql/src/test/results/clientpositive/quotedid_basic.q.out PRE-CREATION > ql/src/test/results/clientpositive/quotedid_partition.q.out PRE-CREATION > ql/src/test/results/clientpositive/quotedid_skew.q.out PRE-CREATION > ql/src/test/results/clientpositive/quotedid_smb.q.out PRE-CREATION > ql/src/test/results/clientpositive/quotedid_tblproperty.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/16299/diff/ > > > Testing > ------- > > added new tests for create, alter, delete, query with columns containing special characters. > Tests start with quotedid > > > Thanks, > > Harish Butani > > --===============2117881694174415293==--