[
https://issues.apache.org/jira/browse/HIVE-27194?focusedWorklogId=854014&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-854014
]
ASF GitHub Bot logged work on HIVE-27194:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Mar/23 20:27
Start Date: 30/Mar/23 20:27
Worklog Time Spent: 10m
Work Description: scarlin-cloudera commented on code in PR #4132:
URL: https://github.com/apache/hive/pull/4132#discussion_r1153752089
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -3914,33 +3914,24 @@ private RelNode genOBLogicalPlan(QB qb, Pair<RelNode,
RowResolver> selPair,
// 1. OB Expr sanity test
// in strict mode, in the presence of order by, limit must be
// specified
- Integer limit = qb.getParseInfo().getDestLimit(dest);
- if (limit == null) {
+ ASTNode limitExpr = qb.getParseInfo().getDestASTLimit(dest);
+ Integer limitValue = qb.getParseInfo().getDestLimit(dest);
+ if (limitExpr == null && limitValue == null) {
String error = StrictChecks.checkNoLimit(conf);
if (error != null) {
throw new
SemanticException(SemanticAnalyzer.generateErrorMessage(obAST, error));
}
}
OBLogicalPlanGenState obLogicalPlanGenState =
beginGenOBLogicalPlan(obAST, selPair, outermostOB);
-
- // 4. Construct SortRel
RelTraitSet traitSet = cluster.traitSetOf(HiveRelNode.CONVENTION);
RelCollation canonizedCollation = traitSet.canonize(
RelCollationImpl.of(obLogicalPlanGenState.getFieldCollation()));
- RelNode sortRel;
- if (limit != null) {
- Integer offset = qb.getParseInfo().getDestLimitOffset(dest);
- RexNode offsetRN = (offset == null || offset == 0) ?
- null :
cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(offset));
- RexNode fetchRN =
cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(limit));
- sortRel = new HiveSortLimit(cluster, traitSet,
obLogicalPlanGenState.getObInputRel(), canonizedCollation,
- offsetRN, fetchRN);
- } else {
- sortRel = new HiveSortLimit(cluster, traitSet,
obLogicalPlanGenState.getObInputRel(), canonizedCollation,
- null, null);
- }
+ RelNode sortRel = genLimitLogicalPlan(qb,
obLogicalPlanGenState.getObInputRel(), canonizedCollation);
+ if (sortRel == null) {
Review Comment:
Is it possible to move this "if" check within genLimitLogicalPlan? Seems
more appropriate to have that method responsible for all creations of the
RelNode.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -3914,33 +3914,24 @@ private RelNode genOBLogicalPlan(QB qb, Pair<RelNode,
RowResolver> selPair,
// 1. OB Expr sanity test
// in strict mode, in the presence of order by, limit must be
// specified
- Integer limit = qb.getParseInfo().getDestLimit(dest);
- if (limit == null) {
+ ASTNode limitExpr = qb.getParseInfo().getDestASTLimit(dest);
Review Comment:
Ok, so I think I have a little bit of a small design preference here that I
like better. Take it for what it's worth...
I like having related variables in one class. We now have two variables
representing limit, the limitValue and limitExpr and they both have to be
checked each time. (offset too).
My preference would be to put this in a class like "LimitOffsetExpr" where
it can hold both the expression and value.
Furthermore, we can place the genValueFromConstantExpr method into that
class to retrieve the calculated expression and removes this method from a way
too cluttered up CalcitePlanner class that desperately needs a rewrite (and
prolly call it something different).
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -1880,10 +1880,20 @@ boolean doPhase1(ASTNode ast, QB qb, Phase1Ctx ctx_1,
PlannerContext plannerCtx)
case HiveParser.TOK_LIMIT:
queryProperties.setHasLimit(true);
if (ast.getChildCount() == 2) {
- qbp.setDestLimit(ctx_1.dest,
- Integer.valueOf(ast.getChild(0).getText()),
Integer.valueOf(ast.getChild(1).getText()));
+ if (ast.getChild(1).getChildCount() == 0 &&
ast.getChild(0).getChildCount() == 0) {
Review Comment:
Along with my above suggestion, you can even move some of this code into the
constructor of the new class.
Issue Time Tracking
-------------------
Worklog Id: (was: 854014)
Time Spent: 20m (was: 10m)
> Support expression in limit and offset clauses
> ----------------------------------------------
>
> Key: HIVE-27194
> URL: https://issues.apache.org/jira/browse/HIVE-27194
> Project: Hive
> Issue Type: Task
> Components: Hive
> Reporter: vamshi kolanu
> Assignee: vamshi kolanu
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> As part of this task, support expressions in both limit and offset clauses.
> Currently, these clauses are only supporting integers.
> For example: The following expressions will be supported after this change.
> 1. select key from (select * from src limit (1+2*3)) q1;
> 2. select key from (select * from src limit (1+2*3) offset (3*4*5)) q1;
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
