----------------------------------------------------------- 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 <https://reviews.apache.org/r/16299/#comment58532> 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 <https://reviews.apache.org/r/16299/#comment58545> Shall we remove this if() altogether and thus also above newly introduced method? ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java <https://reviews.apache.org/r/16299/#comment58546> 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 <https://reviews.apache.org/r/16299/#comment58584> Since this method always return true, no need for this if block. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g <https://reviews.apache.org/r/16299/#comment58585> 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 <https://reviews.apache.org/r/16299/#comment58586> 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 <https://reviews.apache.org/r/16299/#comment58587> 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 <https://reviews.apache.org/r/16299/#comment58588> 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 > >
