-
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 2 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 |
|---|---|---|
|
|
@@ -28,7 +28,6 @@ | |
| import org.apache.hadoop.hive.conf.HiveConf; | ||
| import org.apache.hadoop.hive.ql.ErrorMsg; | ||
| import org.apache.hadoop.hive.ql.exec.ColumnInfo; | ||
| import org.apache.hadoop.hive.ql.exec.FunctionRegistry; | ||
| import org.apache.hadoop.hive.ql.lib.Node; | ||
| import org.apache.hadoop.hive.ql.optimizer.calcite.TraitsUtil; | ||
| import org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveTableFunctionScan; | ||
|
|
@@ -91,14 +90,15 @@ public class LateralViewPlan { | |
|
|
||
| public LateralViewPlan(ASTNode lateralView, RelOptCluster cluster, RelNode inputRel, | ||
| RowResolver inputRR, UnparseTranslator unparseTranslator, | ||
| HiveConf conf, FunctionHelper functionHelper | ||
| ) throws SemanticException { | ||
| HiveConf conf, FunctionHelper functionHelper) throws SemanticException { | ||
| // initialize global variables containing helper information | ||
| this.cluster = cluster; | ||
| this.unparseTranslator = unparseTranslator; | ||
| this.conf = conf; | ||
| this.functionHelper = functionHelper; | ||
|
|
||
| boolean isOuter = lateralView.getToken().getType() == HiveParser.TOK_LATERAL_VIEW_OUTER; | ||
|
|
||
| // AST should have form of LATERAL_VIEW -> SELECT -> SELEXPR -> FUNCTION -> function info tree | ||
| ASTNode selExprAST = (ASTNode) lateralView.getChild(0).getChild(0); | ||
| ASTNode functionAST = (ASTNode) selExprAST.getChild(0); | ||
|
|
@@ -118,7 +118,7 @@ public LateralViewPlan(ASTNode lateralView, RelOptCluster cluster, RelNode input | |
|
|
||
| this.lateralViewRel = HiveTableFunctionScan.create(cluster, | ||
| TraitsUtil.getDefaultTraitSet(cluster), ImmutableList.of(inputRel), udtfCall, | ||
| null, retType, createColumnMappings(inputRel)); | ||
| null, retType, createColumnMappings(inputRel), isOuter); | ||
| } | ||
|
|
||
| public static void validateLateralView(ASTNode lateralView) throws SemanticException { | ||
|
|
@@ -128,8 +128,9 @@ public static void validateLateralView(ASTNode lateralView) throws SemanticExcep | |
| } | ||
| ASTNode next = (ASTNode) lateralView.getChild(1); | ||
| if (!TABLE_ALIAS_TOKEN_TYPES.contains(next.getToken().getType()) && | ||
| HiveParser.TOK_LATERAL_VIEW != next.getToken().getType()) { | ||
| throw new SemanticException(ASTErrorUtils.getMsg( | ||
| HiveParser.TOK_LATERAL_VIEW != next.getToken().getType() && | ||
|
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. I guess now this could also be rewritten using the new helper
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, thanks for catching this! I have refactored this and also other places in SemanticAnalyzer. |
||
| HiveParser.TOK_LATERAL_VIEW_OUTER != next.getToken().getType()) { | ||
| throw new SemanticException(ASTErrorUtils.getMsg( | ||
| ErrorMsg.LATERAL_VIEW_INVALID_CHILD.getMsg(), lateralView)); | ||
| } | ||
| } | ||
|
|
||
| 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.