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




ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
Line 1548 (original), 1548 (patched)
<https://reviews.apache.org/r/72437/#comment309172>

    assert table != null ?
    At this point there is no advantage of passing null for partition keys, 
since if its null then retrieving partitions of this table won't make sense.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Line 832 (original), 830 (patched)
<https://reviews.apache.org/r/72437/#comment309177>

    isView only used for this check here, which can be eliminated.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Lines 1017 (patched)
<https://reviews.apache.org/r/72437/#comment309173>

    assert partitonKeys != null
    If they are null, retrieving partitions won't make sense.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 3473-3474 (patched)
<https://reviews.apache.org/r/72437/#comment309174>

    This needs to be avoided, else it will generate additional RDBMS queries. 
It's only needed for getPartitionsViaSqlFilter() and not on other paths. And 
even there its needed only to make checks which can be avoided altogether. so, 
there is no need for isViewTable anywhere.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 3832 (patched)
<https://reviews.apache.org/r/72437/#comment309175>

    may remove this.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 4077-4078 (patched)
<https://reviews.apache.org/r/72437/#comment309176>

    This needs to be avoided, else it will generate additional RDBMS queries. 
It's only needed for one path and even there its only needed to make checks. 
isViewTable can be eliminated entirely.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 4287 (patched)
<https://reviews.apache.org/r/72437/#comment309178>

    LOG.debug



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 4114 (original), 4307 (patched)
<https://reviews.apache.org/r/72437/#comment309179>

    LOG.debug


- Ashutosh Chauhan


On April 27, 2020, 9:15 a.m., Attila Magyar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72437/
> -----------------------------------------------------------
> 
> (Updated April 27, 2020, 9:15 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan, Rajesh Balamohan, and Vineet Garg.
> 
> 
> Bugs: HIVE-23282
>     https://issues.apache.org/jira/browse/HIVE-23282
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> ObjectStore::getPartitionsByExprInternal internally uses Table information 
> for getting partitionKeys, table, catalog name.
> 
>  
> 
> For this, it ends up populating entire table data from DB (including skew 
> column, parameters, sort, bucket cols etc). This makes it a lot more 
> expensive call. It would be good to check if MTable itself can be used 
> instead of Table.
> 
> 
> Diffs
> -----
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
>  4f58cd91efc 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
>  d1558876f14 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  53b7a67a429 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
>  9834883f00f 
> 
> 
> Diff: https://reviews.apache.org/r/72437/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Attila Magyar
> 
>

Reply via email to