[
https://issues.apache.org/jira/browse/HIVE-784?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13794530#comment-13794530
]
Phabricator commented on HIVE-784:
----------------------------------
ashutoshc has requested changes to the revision "HIVE-784 [jira] Support
uncorrelated subqueries in the WHERE clause".
Design looks good. Mostly implementation related comments.
INLINE COMMENTS
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g:391 It would
be nicer if instead of two rules for IN / NOT IN if we could just have one
rule, which can conditionally generate TOK_SUBQUERY_OP_NOTIN /
TOK_SUBQUERY_OP_IN token. Not a big deal, but would be nice to have since that
makes grammar bit more succinct.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:1804 You
mentioned in comment above that we don't support nested / recursive subq, but I
don't see a check for that. Perhaps, its there but I missed it.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:1789 Thanks
for detailed comments. Very helpful!
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:1811 There
should exactly one subq currently. If so, will be good to add a note for it.
ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java:101 Since, OR
is not supported, It will be good to generate an error message here if OR is
encountered.
ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java:226-233 Same
logic exists in SemanticAnalyzer::doPhase1GetAllAggregations, perhaps we can
create a util method in ParseUtils, instead of repeating code here.
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:289 If you mark
this as transient, you probably wont need to write Kryo serializer for this.
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:699 I dont think
its required. We should probably mark all usage of instances of ASTNodeOrigin
as transient.
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:470 This should
never be the case. Shall we throw an exception here, instead of silently
returning?
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:496 Is this
allowed by standard that subq predicate may refer to Outer? If yes, than in
future perhaps we can add this predicate as a conjunct for outer query.
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:537 Is this need
to be .equals() check here instead of == ?
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:539 It will be
good to add a comment, why we need to have True condition here, instead?
Probably, because plan gen fails later while generating rest of filter plan.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:1821 Good
to name this method as validateAndRewriteAST() ?
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:6526 Quite
a bit of this code is repeated from genJoinTree(), seems like atleast some bits
could be refactored out of genJoinTree() which this method can make use of.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:1827 name
sqOperator is misleading here, topOp perhaps ?
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:41 This is not an
operator in classic Hive sense. Perhaps, SubqASTcontainer or something else.
ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java:24 SubQueryType
instead of SubQueryOperatorType ?
REVISION DETAIL
https://reviews.facebook.net/D13443
BRANCH
SubQuery
ARCANIST PROJECT
hive
To: JIRA, ashutoshc, hbutani
> Support uncorrelated subqueries in the WHERE clause
> ---------------------------------------------------
>
> Key: HIVE-784
> URL: https://issues.apache.org/jira/browse/HIVE-784
> Project: Hive
> Issue Type: New Feature
> Components: Query Processor
> Reporter: Ning Zhang
> Assignee: Harish Butani
> Attachments: D13443.1.patch, HIVE-784.1.patch.txt, HIVE-784.2.patch,
> SubQuerySpec.pdf, tpchQueriesUsingSubQueryClauses.sql
>
>
> Hive currently only support views in the FROM-clause, some Facebook use cases
> suggest that Hive should support subqueries such as those connected by
> IN/EXISTS in the WHERE-clause.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
