> On Dec. 18, 2013, 12:59 a.m., Ashutosh Chauhan wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 872 > > <https://reviews.apache.org/r/16299/diff/2/?file=398469#file398469line872> > > > > 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.
done > On Dec. 18, 2013, 12:59 a.m., Ashutosh Chauhan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java, > > line 484 > > <https://reviews.apache.org/r/16299/diff/2/?file=398471#file398471line484> > > > > Shall we remove this if() altogether and thus also above newly > > introduced method? i kept the validateColumnName method around in case we decide to change the validation logic in the future. But if you feel strongly about it, i will remove it. > On Dec. 18, 2013, 12:59 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java, line 283 > > <https://reviews.apache.org/r/16299/diff/2/?file=398472#file398472line283> > > > > 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. done > On Dec. 18, 2013, 12:59 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 34 > > <https://reviews.apache.org/r/16299/diff/2/?file=398475#file398475line34> > > > > There can never be the case that hiveconf == null. That will be a bug. > > Lets remove this null check. done > On Dec. 18, 2013, 12:59 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 400 > > <https://reviews.apache.org/r/16299/diff/2/?file=398475#file398475line400> > > > > It will be good to document where all Identifier is used. Can be lifted > > straight from your html document. > > done > On Dec. 18, 2013, 12:59 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 403 > > <https://reviews.apache.org/r/16299/diff/2/?file=398475#file398475line403> > > > > Good to add a note here saying QuotedIdentifier only optionally > > available for columns as of now. done - Harish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16299/#review30570 ----------------------------------------------------------- On Dec. 18, 2013, 5:42 p.m., Harish Butani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16299/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2013, 5:42 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 971c5d3 > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java > 5b75ef3 > 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 > dbf3f91 > 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 b9cd65c > 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 > >
