You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vihang Karajgaonkar <vi...@cloudera.com> on 2017/05/30 21:42:58 UTC

Review Request 59654: HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database

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

Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.


Repository: hive-git


Description
-------

HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database


Diffs
-----

  beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java a4ecc089f8a19bd94af2eae17e534cc6e8d2a9ca 
  beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java f312e46d755fc843369167af6d4da3de7dbe4a61 
  beeline/src/test/org/apache/hive/beeline/TestHiveSchemaTool.java 716bce7289b6e7c4d4bcea38b06f591a9ad9c098 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java d1658f7bb2e5306b6b5954165759a217d1ba235c 
  metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreSchemaInfo.java d6627433cb0f3f1c8fe868b3748fd754a7bff821 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 6bddb8e864d26a6cb7e2fe9b8b8becf427779944 


Diff: https://reviews.apache.org/r/59654/diff/1/


Testing
-------


Thanks,

Vihang Karajgaonkar


Re: Review Request 59654: HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database

Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59654/#review176369
-----------------------------------------------------------


Fix it, then Ship it!




Overall, LGTM.


beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java
Line 784 (original), 759-760 (patched)
<https://reviews.apache.org/r/59654/#comment249752>

    Previously, `baseDir.getParent()` was used in when constructing `schemaFile`, but it seems that it is no longer the case.
    
    Probably safer to use the `File(String, String)` constructor instead of building up the file name using `File.separatorChar` since we don't know what metaStoreSchemaInfo.getMetaStoreScriptDir() and metaStoreSchemaInfo.generateInitFileName(version) return.


- Sahil Takiar


On May 30, 2017, 9:43 p.m., Vihang Karajgaonkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59654/
> -----------------------------------------------------------
> 
> (Updated May 30, 2017, 9:43 p.m.)
> 
> 
> Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.
> 
> 
> Bugs: HIVE-16771
>     https://issues.apache.org/jira/browse/HIVE-16771
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database
> 
> 
> Diffs
> -----
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java a4ecc089f8a19bd94af2eae17e534cc6e8d2a9ca 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java f312e46d755fc843369167af6d4da3de7dbe4a61 
>   beeline/src/test/org/apache/hive/beeline/TestHiveSchemaTool.java 716bce7289b6e7c4d4bcea38b06f591a9ad9c098 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java d1658f7bb2e5306b6b5954165759a217d1ba235c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreSchemaInfo.java d6627433cb0f3f1c8fe868b3748fd754a7bff821 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 6bddb8e864d26a6cb7e2fe9b8b8becf427779944 
> 
> 
> Diff: https://reviews.apache.org/r/59654/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>


Re: Review Request 59654: HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59654/#review176375
-----------------------------------------------------------


Ship it!




The patch looks good. I agree this needs more refactoring to make it better, but it is more work to do.
Let's ship this for now.

- Sergio Pena


On May 30, 2017, 9:43 p.m., Vihang Karajgaonkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59654/
> -----------------------------------------------------------
> 
> (Updated May 30, 2017, 9:43 p.m.)
> 
> 
> Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.
> 
> 
> Bugs: HIVE-16771
>     https://issues.apache.org/jira/browse/HIVE-16771
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database
> 
> 
> Diffs
> -----
> 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java a4ecc089f8a19bd94af2eae17e534cc6e8d2a9ca 
>   beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java f312e46d755fc843369167af6d4da3de7dbe4a61 
>   beeline/src/test/org/apache/hive/beeline/TestHiveSchemaTool.java 716bce7289b6e7c4d4bcea38b06f591a9ad9c098 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java d1658f7bb2e5306b6b5954165759a217d1ba235c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreSchemaInfo.java d6627433cb0f3f1c8fe868b3748fd754a7bff821 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 6bddb8e864d26a6cb7e2fe9b8b8becf427779944 
> 
> 
> Diff: https://reviews.apache.org/r/59654/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>


Re: Review Request 59654: HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database

Posted by Vihang Karajgaonkar <vi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59654/
-----------------------------------------------------------

(Updated May 30, 2017, 11:22 p.m.)


Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.


Changes
-------

Added review suggestions


Bugs: HIVE-16771
    https://issues.apache.org/jira/browse/HIVE-16771


Repository: hive-git


Description
-------

HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database


Diffs (updated)
-----

  beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java a4ecc089f8a19bd94af2eae17e534cc6e8d2a9ca 
  beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java f312e46d755fc843369167af6d4da3de7dbe4a61 
  beeline/src/test/org/apache/hive/beeline/TestHiveSchemaTool.java 716bce7289b6e7c4d4bcea38b06f591a9ad9c098 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java d1658f7bb2e5306b6b5954165759a217d1ba235c 
  metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreSchemaInfo.java d6627433cb0f3f1c8fe868b3748fd754a7bff821 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 6bddb8e864d26a6cb7e2fe9b8b8becf427779944 


Diff: https://reviews.apache.org/r/59654/diff/2/

Changes: https://reviews.apache.org/r/59654/diff/1-2/


Testing
-------


Thanks,

Vihang Karajgaonkar


Re: Review Request 59654: HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database

Posted by Vihang Karajgaonkar <vi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59654/
-----------------------------------------------------------

(Updated May 30, 2017, 9:43 p.m.)


Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.


Changes
-------

added the link to JIRA


Bugs: HIVE-16771
    https://issues.apache.org/jira/browse/HIVE-16771


Repository: hive-git


Description
-------

HIVE-16771 : Schematool should use MetastoreSchemaInfo to get the metastore schema version from database


Diffs
-----

  beeline/src/java/org/apache/hive/beeline/HiveSchemaHelper.java a4ecc089f8a19bd94af2eae17e534cc6e8d2a9ca 
  beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java f312e46d755fc843369167af6d4da3de7dbe4a61 
  beeline/src/test/org/apache/hive/beeline/TestHiveSchemaTool.java 716bce7289b6e7c4d4bcea38b06f591a9ad9c098 
  itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java d1658f7bb2e5306b6b5954165759a217d1ba235c 
  metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreSchemaInfo.java d6627433cb0f3f1c8fe868b3748fd754a7bff821 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java 6bddb8e864d26a6cb7e2fe9b8b8becf427779944 


Diff: https://reviews.apache.org/r/59654/diff/1/


Testing
-------


Thanks,

Vihang Karajgaonkar