hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Szehon Ho <sze...@cloudera.com>
Subject Re: Review Request 48233: HIVE-13884: Disallow queries fetching more than a configured number of partitions in PartitionPruner
Date Thu, 16 Jun 2016 21:24:30 GMT

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 780)
<https://reviews.apache.org/r/48233/#comment203308>

    Should we add this to 'metaVars' variable?  Reading the doc, it seems it will affect HiveCLI
and allow those users to change it on the fly.



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 3197)
<https://reviews.apache.org/r/48233/#comment203304>

    The "> -1" is not strictly needed as it was already checked earlier by isPartitionLimitEnabled.
    
    To be clearer, we should have this method just start with: 
    
    if !isPartitionLimitEnabled() {
      return;
      
    that way we don't have to have the extra checks around this method.



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 4792)
<https://reviews.apache.org/r/48233/#comment203302>

    Should fix this?


- Szehon Ho


On June 16, 2016, 4:04 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48233/
> -----------------------------------------------------------
> 
> (Updated June 16, 2016, 4:04 p.m.)
> 
> 
> Review request for hive, Mohit Sabharwal and Naveen Gangam.
> 
> 
> Bugs: HIVE-13884
>     https://issues.apache.org/jira/browse/HIVE-13884
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> The patch verifies the # of partitions a table has before fetching any from the metastore.
I
> t checks that limit from 'hive.limit.query.max.table.partition'.
> 
> A limitation added here is that the variable must be on hive-site.xml in order to work,
and it does not accept to set this through beeline because HiveMetaStore.java does not read
the variables set through beeline. I think it is better to keep it this way to avoid users
changing the value on fly, and crashing the metastore.
> 
> Another change is that EXPLAIN commands won't be executed either. EXPLAIN commands need
to fetch partitions in order to create the operator tree. If we allow EXPLAIN to do that,
then we may have the same OOM situations for large partitions.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 761dbb279fb196e2bf1e0e59824827a4504eb136

>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c0827ea9d47e569d9697649a7e16d196de3de14d

>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java c135179b97354108f842a5ca2de0c6f0ef28b7fc

>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java da188d33d6194740ba9ecb37a6e533ecf1ec6906

>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java a6d3f5385b33b8a4e31ee20ca5cb8f58c97c8702

>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 31f0d7b89670b8a749bbe8a7ff2b4ff9f059a8e2

>   metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
3152e77c3c7152ac4dbe7e779ce35f28044fe3c9 
>   metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
86a243609b23e2ca9bb8849f0da863a95e477d5c 
> 
> Diff: https://reviews.apache.org/r/48233/diff/
> 
> 
> Testing
> -------
> 
> Waiting for HiveQA.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message