[
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
