You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Peter Vary via Review Board <no...@reviews.apache.org> on 2018/01/22 13:40:54 UTC
Review Request 65265: HIVE-18509 Create tests for table manipulation
related methods (create, alter, drop)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/
-----------------------------------------------------------
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Bugs: HIVE-18509
https://issues.apache.org/jira/browse/HIVE-18509
Repository: hive-git
Description
-------
Added the new tests.
Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
Diffs
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
Diff: https://reviews.apache.org/r/65265/diff/1/
Testing
-------
Run the tests
Thanks,
Peter Vary
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/#review196253
-----------------------------------------------------------
Ship it!
Ship It!
- Marta Kuczora
On Jan. 23, 2018, 4:54 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65265/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 4:54 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18509
> https://issues.apache.org/jira/browse/HIVE-18509
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Added the new tests.
> Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
>
>
> Diff: https://reviews.apache.org/r/65265/diff/4/
>
>
> Testing
> -------
>
> Run the tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/
-----------------------------------------------------------
(Updated Jan. 23, 2018, 4:54 p.m.)
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Changes
-------
Addressed Adam's comments
Bugs: HIVE-18509
https://issues.apache.org/jira/browse/HIVE-18509
Repository: hive-git
Description
-------
Added the new tests.
Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
Diffs (updated)
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
Diff: https://reviews.apache.org/r/65265/diff/4/
Changes: https://reviews.apache.org/r/65265/diff/3-4/
Testing
-------
Run the tests
Thanks,
Peter Vary
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On Jan. 23, 2018, 4:17 p.m., Adam Szita wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
> > Lines 234-235 (patched)
> > <https://reviews.apache.org/r/65265/diff/3/?file=1944444#file1944444line234>
> >
> > Could use assertEquals here instead IMHO
Changed. Thanks!
> On Jan. 23, 2018, 4:17 p.m., Adam Szita wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
> > Lines 429 (patched)
> > <https://reviews.apache.org/r/65265/diff/3/?file=1944444#file1944444line429>
> >
> > Is this line of code necessary here?
Removed :)
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/#review196012
-----------------------------------------------------------
On Jan. 23, 2018, 4:54 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65265/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 4:54 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18509
> https://issues.apache.org/jira/browse/HIVE-18509
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Added the new tests.
> Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
>
>
> Diff: https://reviews.apache.org/r/65265/diff/4/
>
>
> Testing
> -------
>
> Run the tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Adam Szita via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/#review196012
-----------------------------------------------------------
Fix it, then Ship it!
Looking good, thanks Peter!
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
Lines 234-235 (patched)
<https://reviews.apache.org/r/65265/#comment275544>
Could use assertEquals here instead IMHO
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
Lines 429 (patched)
<https://reviews.apache.org/r/65265/#comment275528>
Is this line of code necessary here?
- Adam Szita
On Jan. 23, 2018, 11:48 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65265/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:48 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18509
> https://issues.apache.org/jira/browse/HIVE-18509
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Added the new tests.
> Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
>
>
> Diff: https://reviews.apache.org/r/65265/diff/3/
>
>
> Testing
> -------
>
> Run the tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/
-----------------------------------------------------------
(Updated Jan. 23, 2018, 11:48 a.m.)
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Changes
-------
Addressed Marta's comments
Bugs: HIVE-18509
https://issues.apache.org/jira/browse/HIVE-18509
Repository: hive-git
Description
-------
Added the new tests.
Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
Diffs (updated)
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
Diff: https://reviews.apache.org/r/65265/diff/3/
Changes: https://reviews.apache.org/r/65265/diff/2-3/
Testing
-------
Run the tests
Thanks,
Peter Vary
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On Jan. 23, 2018, 11:06 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
> > Lines 323 (patched)
> > <https://reviews.apache.org/r/65265/diff/2/?file=1943732#file1943732line323>
> >
> > Maybe this could be moved to a separate test case (like create table with null location). It was a bit hard to notice after the asserts of the previous test case.
Done
> On Jan. 23, 2018, 11:06 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
> > Lines 505 (patched)
> > <https://reviews.apache.org/r/65265/diff/2/?file=1943732#file1943732line505>
> >
> > It might be good to check if the table is really deleted.
Done. Thanks for pointing this out!
> On Jan. 23, 2018, 11:06 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
> > Lines 602 (patched)
> > <https://reviews.apache.org/r/65265/diff/2/?file=1943732#file1943732line602>
> >
> > This test contains three well defined use cases so I think it would make sense to split them into 3 tests.
Done
> On Jan. 23, 2018, 11:06 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
> > Lines 911 (patched)
> > <https://reviews.apache.org/r/65265/diff/2/?file=1943732#file1943732line911>
> >
> > What happens if the location of the table is changed to null? Is it a valid use case or an exception will occur?
Good idea. Done
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/#review195980
-----------------------------------------------------------
On Jan. 23, 2018, 11:48 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65265/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:48 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18509
> https://issues.apache.org/jira/browse/HIVE-18509
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Added the new tests.
> Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
>
>
> Diff: https://reviews.apache.org/r/65265/diff/3/
>
>
> Testing
> -------
>
> Run the tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/#review195980
-----------------------------------------------------------
Thanks a lot Peter for the patch.
It looks really good to me, I have only few nits.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
Lines 323 (patched)
<https://reviews.apache.org/r/65265/#comment275450>
Maybe this could be moved to a separate test case (like create table with null location). It was a bit hard to notice after the asserts of the previous test case.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
Lines 505 (patched)
<https://reviews.apache.org/r/65265/#comment275451>
It might be good to check if the table is really deleted.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
Lines 602 (patched)
<https://reviews.apache.org/r/65265/#comment275452>
This test contains three well defined use cases so I think it would make sense to split them into 3 tests.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
Lines 911 (patched)
<https://reviews.apache.org/r/65265/#comment275453>
What happens if the location of the table is changed to null? Is it a valid use case or an exception will occur?
- Marta Kuczora
On Jan. 22, 2018, 3:36 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65265/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2018, 3:36 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18509
> https://issues.apache.org/jira/browse/HIVE-18509
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Added the new tests.
> Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
>
>
> Diff: https://reviews.apache.org/r/65265/diff/2/
>
>
> Testing
> -------
>
> Run the tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65265: HIVE-18509 Create tests for table
manipulation related methods (create, alter, drop)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65265/
-----------------------------------------------------------
(Updated Jan. 22, 2018, 3:36 p.m.)
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Changes
-------
Addressed checkstyle errors
Bugs: HIVE-18509
https://issues.apache.org/jira/browse/HIVE-18509
Repository: hive-git
Description
-------
Added the new tests.
Modified AbstractMetaStoreService, so when starting mini-metastore it is possible to provide configuration value overrides for non-metastore config values (like fs trash settings)
Diffs (updated)
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java PRE-CREATION
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java ed071f8
Diff: https://reviews.apache.org/r/65265/diff/2/
Changes: https://reviews.apache.org/r/65265/diff/1-2/
Testing
-------
Run the tests
Thanks,
Peter Vary