You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by pengcheng xiong <px...@hortonworks.com> on 2016/05/06 01:38:30 UTC

Review Request 47051: Trailing spaces in partition column will be treated differently

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47051/
-----------------------------------------------------------

Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-13618


Diffs
-----

  itests/src/test/resources/testconfiguration.properties 88381aa 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 5c49be9 
  ql/src/test/queries/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
  ql/src/test/queries/clientpositive/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
  ql/src/test/results/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/tez/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/47051/diff/


Testing
-------


Thanks,

pengcheng xiong


Re: Review Request 47051: Trailing spaces in partition column will be treated differently

Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47051/
-----------------------------------------------------------

(Updated May 6, 2016, 10:25 p.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
-------

HIVE-13618


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 88381aa 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 5c49be9 
  ql/src/test/queries/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
  ql/src/test/queries/clientpositive/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
  ql/src/test/results/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/tez/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/47051/diff/


Testing
-------


Thanks,

pengcheng xiong


Re: Review Request 47051: Trailing spaces in partition column will be treated differently

Posted by pengcheng xiong <px...@hortonworks.com>.

> On May 6, 2016, 1:51 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, lines 1834-1836
> > <https://reviews.apache.org/r/47051/diff/1/?file=1374301#file1374301line1834>
> >
> >     I think better comment could be something like following:
> >     
> >     // We need to compare partition name with requested name since some DBs (like MySQL, Derby) considers 'a' = 'a ' whereas others like (Postgres, Oracle) doesn't exhibit this problem.

OK.


> On May 6, 2016, 1:51 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line 1839
> > <https://reviews.apache.org/r/47051/diff/1/?file=1374301#file1374301line1839>
> >
> >     Does this need equalsIgnoreCase() ?

it is not necessary. it is stored as lowercases.


> On May 6, 2016, 1:51 a.m., Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line 1838
> > <https://reviews.apache.org/r/47051/diff/1/?file=1374301#file1374301line1838>
> >
> >     We should throw exception if mparts.size() > 1

If we throw exception, then there is no need to make an exact comparison later.


- pengcheng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47051/#review131980
-----------------------------------------------------------


On May 6, 2016, 1:38 a.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47051/
> -----------------------------------------------------------
> 
> (Updated May 6, 2016, 1:38 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13618
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 88381aa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 5c49be9 
>   ql/src/test/queries/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
>   ql/src/test/results/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/tez/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47051/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>


Re: Review Request 47051: Trailing spaces in partition column will be treated differently

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47051/#review131980
-----------------------------------------------------------




metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (lines 1833 - 1835)
<https://reviews.apache.org/r/47051/#comment196021>

    I think better comment could be something like following:
    
    // We need to compare partition name with requested name since some DBs (like MySQL, Derby) considers 'a' = 'a ' whereas others like (Postgres, Oracle) doesn't exhibit this problem.



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 1837)
<https://reviews.apache.org/r/47051/#comment196023>

    We should throw exception if mparts.size() > 1



metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 1838)
<https://reviews.apache.org/r/47051/#comment196022>

    Does this need equalsIgnoreCase() ?


- Ashutosh Chauhan


On May 6, 2016, 1:38 a.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47051/
> -----------------------------------------------------------
> 
> (Updated May 6, 2016, 1:38 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13618
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 88381aa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 5c49be9 
>   ql/src/test/queries/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/partition_column_names_with_leading_and_trailing_spaces.q PRE-CREATION 
>   ql/src/test/results/clientnegative/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/tez/partition_column_names_with_leading_and_trailing_spaces.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47051/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>