hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phabricator (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HIVE-4914) filtering via partition name should be done inside metastore server (implementation)
Date Fri, 30 Aug 2013 13:30:52 GMT

    [ https://issues.apache.org/jira/browse/HIVE-4914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13754685#comment-13754685
] 

Phabricator commented on HIVE-4914:
-----------------------------------

ashutoshc has requested changes to the revision "HIVE-4914 [jira] filtering via partition
name should be done inside metastore server (implementation)".

  Comments.

INLINE COMMENTS
  metastore/if/hive_metastore.thrift:282 Can you add a comment here defining this boolean?
  metastore/if/hive_metastore.thrift:510 Instead of list of parameters, can you define a struct
which is passed in as an argument. That way in future if we need to add another parameter
for this function, it will still be back-compat.
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:252 Consider using
MetaStoreUtils::newInstance() for this.
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:249 Can you add a comment
here saying this class is created via reflection to avoid circular dependency on ql package?
  metastore/if/hive_metastore.thrift:281  Set<Partition> instead of list<Partition>
?
  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:569 Why not enhance existing deserializeExpressions()
to allow it to throw exception? Or, atleast reuse the common code.
  metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:494 Add description
of @param FilterBuilder in javadoc here.
  metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:445 Update
javadoc with new param.
  metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:418 Update
javadoc.
  metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:113 ExpressionTree
is getting too large. Better to put this class and FilterBuilder in another file?
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:1887 With query.setRange()
this is no longer required.
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:1944 Didn't get this.
With this patch, Hive client wont do any work, right?
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:1971 This TODO is important
to resolve. Can you follow up on this?
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:1965 Is this just for
tests? Or is it needed? Either way, can you add a comment for it.
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java:5756 This class is
getting too large. May be a good idea to put some of the helper inner classes and methods
in MetaStoreUtils class.
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java:496 This class
is also growing in size. Probably, put it in a seprate file along with TreeVisitor.

REVISION DETAIL
  https://reviews.facebook.net/D12561

BRANCH
  HIVE-4914-no-gen

ARCANIST PROJECT
  hive

To: JIRA, ashutoshc, sershe

                
> filtering via partition name should be done inside metastore server (implementation)
> ------------------------------------------------------------------------------------
>
>                 Key: HIVE-4914
>                 URL: https://issues.apache.org/jira/browse/HIVE-4914
>             Project: Hive
>          Issue Type: Improvement
>          Components: Metastore
>            Reporter: Sergey Shelukhin
>            Assignee: Sergey Shelukhin
>         Attachments: HIVE-4914.01.patch, HIVE-4914.D12561.1.patch, HIVE-4914-only-no-gen.patch,
HIVE-4914-only.patch, HIVE-4914.patch, HIVE-4914.patch, HIVE-4914.patch
>
>
> Currently, if the filter pushdown is impossible (which is most cases), the client gets
all partition names from metastore, filters them, and asks for partitions by names for the
filtered set.
> Metastore server code should do that instead; it should check if pushdown is possible
and do it if so; otherwise it should do name-based filtering.
> Saves the roundtrip with all partition names from the server to client, and also removes
the need to have pushdown viability checking on both sides.
> NO PRECOMMIT TESTS

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message