You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Marta Kuczora via Review Board <no...@reviews.apache.org> on 2018/03/07 15:48:27 UTC
Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/
-----------------------------------------------------------
Review request for hive, Peter Vary and Adam Szita.
Bugs: HIVE-18898
https://issues.apache.org/jira/browse/HIVE-18898
Repository: hive-git
Description
-------
The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
Diffs
-----
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
Diff: https://reviews.apache.org/r/65952/diff/1/
Testing
-------
Run the TestDropPartitions tests.
Thanks,
Marta Kuczora
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
> On March 8, 2018, 2:15 a.m., Alexander Kolbasov wrote:
> > Afrew with Sahil - the checks should be on the server side.
Thanks a lot Sasha for the review!
I absolutely agree that it would be better to have the checks on the server side, however these checks have to be done in the client, because the NPE occurs before calling HiveMetaStore.
As I wrote above, the PartitionDropOptions parameter is not sent to HiveMetaStore. In HiveMetaStore we only have the boolean values which are set in PartitionDropOptions and the NPE occurs when trying to get these variables.
With a part_vals list which has null values, the NPE occurs when thrift is trying to serialize it. So we should do the check before calling the thrift method.
- Marta
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review198847
-----------------------------------------------------------
On March 7, 2018, 3:48 p.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 7, 2018, 3:48 p.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/1/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review198847
-----------------------------------------------------------
Afrew with Sahil - the checks should be on the server side.
- Alexander Kolbasov
On March 7, 2018, 3:48 p.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 7, 2018, 3:48 p.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/1/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review199572
-----------------------------------------------------------
Ship it!
Ship It!
- Sahil Takiar
On March 8, 2018, 9:40 a.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 8, 2018, 9:40 a.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 662de9a
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/3/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
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/65952/
-----------------------------------------------------------
(Updated March 8, 2018, 9:40 a.m.)
Review request for hive, Peter Vary and Adam Szita.
Changes
-------
Fixed stlyecheck issue.
Bugs: HIVE-18898
https://issues.apache.org/jira/browse/HIVE-18898
Repository: hive-git
Description
-------
The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
Diffs (updated)
-----
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 662de9a
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
Diff: https://reviews.apache.org/r/65952/diff/3/
Changes: https://reviews.apache.org/r/65952/diff/2-3/
Testing
-------
Run the TestDropPartitions tests.
Thanks,
Marta Kuczora
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
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/65952/
-----------------------------------------------------------
(Updated March 8, 2018, 9:35 a.m.)
Review request for hive, Peter Vary and Adam Szita.
Changes
-------
Added checks for the db and table names and for the part value list.
Bugs: HIVE-18898
https://issues.apache.org/jira/browse/HIVE-18898
Repository: hive-git
Description
-------
The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
Diffs (updated)
-----
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 662de9a
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
Diff: https://reviews.apache.org/r/65952/diff/2/
Changes: https://reviews.apache.org/r/65952/diff/1-2/
Testing
-------
Run the TestDropPartitions tests.
Thanks,
Marta Kuczora
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
> On March 7, 2018, 5:06 p.m., Sahil Takiar wrote:
> >
Thanks a lot Sahil for the review.
> On March 7, 2018, 5:06 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 996 (patched)
> > <https://reviews.apache.org/r/65952/diff/1/?file=1972255#file1972255line996>
> >
> > Why do this validation in `HiveMetaStoreClient` rather than `HiveMetaStore`?
I did it there, because the PartitionDropOptions parameter is not sent to HiveMetaStore, only the boolean values from it.
The dropPartition method in the HiveMetaStoreClient looked like this:
public boolean dropPartition(String db_name, String tbl_name,
List<String> part_vals, PartitionDropOptions options) throws TException {
return dropPartition(db_name, tbl_name, part_vals, options.deleteData,
options.purgeData? getEnvironmentContextWithIfPurgeSet() : null);
}
And the NPE occurred when called options.deleteData in the dropPartition method call.
> On March 7, 2018, 5:06 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 1006 (patched)
> > <https://reviews.apache.org/r/65952/diff/1/?file=1972255#file1972255line1006>
> >
> > Do the other parameters need to be checked?
You are right, we can check the other parameters too.
- Marta
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review198796
-----------------------------------------------------------
On March 7, 2018, 3:48 p.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 7, 2018, 3:48 p.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/1/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
> On March 7, 2018, 5:06 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 1006 (patched)
> > <https://reviews.apache.org/r/65952/diff/1/?file=1972255#file1972255line1006>
> >
> > Do the other parameters need to be checked?
>
> Marta Kuczora wrote:
> You are right, we can check the other parameters too.
Added checks to HiveMetaStore for the db and table names and for the part_vals list.
- Marta
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review198796
-----------------------------------------------------------
On March 8, 2018, 9:35 a.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 8, 2018, 9:35 a.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 662de9a
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/2/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Sahil Takiar <ta...@gmail.com>.
> On March 7, 2018, 5:06 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 996 (patched)
> > <https://reviews.apache.org/r/65952/diff/1/?file=1972255#file1972255line996>
> >
> > Why do this validation in `HiveMetaStoreClient` rather than `HiveMetaStore`?
>
> Marta Kuczora wrote:
> I did it there, because the PartitionDropOptions parameter is not sent to HiveMetaStore, only the boolean values from it.
> The dropPartition method in the HiveMetaStoreClient looked like this:
>
> public boolean dropPartition(String db_name, String tbl_name,
> List<String> part_vals, PartitionDropOptions options) throws TException {
>
> return dropPartition(db_name, tbl_name, part_vals, options.deleteData,
> options.purgeData? getEnvironmentContextWithIfPurgeSet() : null);
> }
>
> And the NPE occurred when called options.deleteData in the dropPartition method call.
Makes sense, thanks for the information Marta.
- Sahil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review198796
-----------------------------------------------------------
On March 8, 2018, 9:40 a.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 8, 2018, 9:40 a.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 662de9a
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/3/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>
Re: Review Request 65952: HIVE-18898: Fix NPEs in
HiveMetastore.dropPartition method
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65952/#review198796
-----------------------------------------------------------
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
Lines 996 (patched)
<https://reviews.apache.org/r/65952/#comment279042>
Why do this validation in `HiveMetaStoreClient` rather than `HiveMetaStore`?
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
Lines 1006 (patched)
<https://reviews.apache.org/r/65952/#comment279043>
Do the other parameters need to be checked?
- Sahil Takiar
On March 7, 2018, 3:48 p.m., Marta Kuczora wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65952/
> -----------------------------------------------------------
>
> (Updated March 7, 2018, 3:48 p.m.)
>
>
> Review request for hive, Peter Vary and Adam Szita.
>
>
> Bugs: HIVE-18898
> https://issues.apache.org/jira/browse/HIVE-18898
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> The TestDropPartitions tests revealed that NPE is thrown if the dropPartition(String db_name, String tbl_name, List<String> part_vals, PartitionDropOptions options) method is called with null options and with a part_vals list which contains null elements.
>
>
> Diffs
> -----
>
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 3128089
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java 4d94ebf
>
>
> Diff: https://reviews.apache.org/r/65952/diff/1/
>
>
> Testing
> -------
>
> Run the TestDropPartitions tests.
>
>
> Thanks,
>
> Marta Kuczora
>
>