-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29612: LATERAL VIEW OUTER doesn't work with CBO enabled #6482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f2ec801
da242cd
544b33f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1683,8 +1683,7 @@ private void processJoin(QB qb, ASTNode join) throws SemanticException { | |
| "PTF invocation in a Join must have an alias")); | ||
| } | ||
|
|
||
| } else if (child.getToken().getType() == HiveParser.TOK_LATERAL_VIEW || | ||
| child.getToken().getType() == HiveParser.TOK_LATERAL_VIEW_OUTER) { | ||
| } else if (isASTNodeLateralViewOrOuter(child)) { | ||
| // SELECT * FROM src1 LATERAL VIEW udtf() AS myTable JOIN src2 ... | ||
| // is not supported. Instead, the lateral view must be in a subquery | ||
| // SELECT * FROM (SELECT * FROM src1 LATERAL VIEW udtf() AS myTable) a | ||
|
|
@@ -1713,9 +1712,7 @@ private String processLateralView(QB qb, ASTNode lateralView) | |
| int numChildren = lateralView.getChildCount(); | ||
| assert (numChildren == 2); | ||
|
|
||
| if (!isCBOSupportedLateralView(lateralView)) { | ||
| queryProperties.setCBOSupportedLateralViews(false); | ||
| } | ||
| queryProperties.setCBOSupportedLateralViews(isCBOSupportedLateralView()); | ||
|
|
||
| ASTNode next = (ASTNode) lateralView.getChild(1); | ||
| String alias = null; | ||
|
|
@@ -1889,8 +1886,7 @@ boolean doPhase1(ASTNode ast, QB qb, Phase1Ctx ctx_1, PlannerContext plannerCtx, | |
| processTable(qb, frm); | ||
| } else if (frm.getToken().getType() == HiveParser.TOK_SUBQUERY) { | ||
| processSubQuery(qb, frm); | ||
| } else if (frm.getToken().getType() == HiveParser.TOK_LATERAL_VIEW || | ||
| frm.getToken().getType() == HiveParser.TOK_LATERAL_VIEW_OUTER) { | ||
| } else if (isASTNodeLateralViewOrOuter(frm)) { | ||
| queryProperties.setHasLateralViews(true); | ||
| processLateralView(qb, frm); | ||
| } else if (isJoinToken(frm)) { | ||
|
|
@@ -11123,7 +11119,7 @@ boolean isCBOExecuted() { | |
| return false; | ||
| } | ||
|
|
||
| boolean isCBOSupportedLateralView(ASTNode lateralView) { | ||
| boolean isCBOSupportedLateralView() { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -15526,4 +15522,14 @@ public void startAnalysis() { | |
| queryState.createHMSCache(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Utility method to determine if an AST node represents a lateral view or lateral view outer. | ||
| * @param node AST node | ||
| * @return true if node is of lateral view or lateral view outer; false otherwise. | ||
| */ | ||
| public static boolean isASTNodeLateralViewOrOuter(ASTNode node) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit.: I think we can name this just simply
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I can rename this method in the next commit. 👍🏼 |
||
| return node.getToken().getType() == HiveParser.TOK_LATERAL_VIEW | ||
| || node.getToken().getType() == HiveParser.TOK_LATERAL_VIEW_OUTER; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| CREATE TABLE test (id string, items array<string>); | ||
| INSERT INTO test VALUES ('A', array('a', 'b')), ('B', array('c')), ('D', array()); | ||
|
|
||
| CREATE VIEW v AS | ||
| SELECT test.id AS id, item | ||
| FROM test | ||
| LATERAL VIEW OUTER explode(test.items) lv AS item; | ||
|
|
||
| -- CBO plan should contain `outer=[true]` in HiveTableFunctionScan node. | ||
| EXPLAIN CBO | ||
| SELECT id, item FROM v ORDER BY id, item; | ||
| -- Explain plan should contain `outer lateral view: true` in the UDTF Operator | ||
| EXPLAIN | ||
| SELECT id, item FROM v ORDER BY id, item; | ||
| -- One of the output row should be ('D', NULL) since it's an outer lateral view. | ||
| SELECT id, item FROM v ORDER BY id, item; |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the result didn't depend on the
HIVE_CBO_RETPATH_HIVEOPconf option. Maybe it should still return true iflateralView.getToken().getType() == HiveParser.TOK_LATERAL_VIEW?Could you please explain why using that conf option here? It's description is
Flag to control calcite plan to hive operator conversion, but I don't understand its purpose.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomasrebele
CBO return path is a different implementation of transforming the logical plan to Hive operators. It skips the AST conversion and converts Calcite operators directly to Hive operators. Maybe I'm missing something but I haven't found any change regarding this conversion in this patch.
@soumyakanti3578
Does lateral views in general is supported in CBO return path?
If yes could you please implement the lateral view outer too. I assume it would affect only 1-2 files and it would fit into the scope of this patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither
LATERAL VIEWnorLATERAL VIEW OUTERwork with CBO return path. I'll create a separate ticket for them, as I don't think there's a ticket for them here: https://issues.apache.org/jira/browse/HIVE-9132So the current condition in
isCBOSupportedLateralView()is true for both types ofLATERAL VIEWwhen CBO is enabled (If CBO is not enabled we take a different path), and it is false only whenHIVE_CBO_RETPATH_HIVEOPis true (non default path for CBO). So I think the condition correctly reflects the current state.