You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hive.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2023/03/30 16:33:00 UTC
[jira] [Work logged] (HIVE-27194) Support expression in limit and offset clauses
[ https://issues.apache.org/jira/browse/HIVE-27194?focusedWorklogId=853965&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-853965 ]
ASF GitHub Bot logged work on HIVE-27194:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Mar/23 16:32
Start Date: 30/Mar/23 16:32
Worklog Time Spent: 10m
Work Description: jfsii commented on code in PR #4132:
URL: https://github.com/apache/hive/pull/4132#discussion_r1153499754
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -4179,30 +4170,53 @@ public RelNode endGenOBLogicalPlan(OBLogicalPlanGenState obLogicalPlanGenState,
}
}
- private RelNode genLimitLogicalPlan(QB qb, RelNode srcRel) throws SemanticException {
- HiveRelNode sortRel = null;
- QBParseInfo qbp = getQBParseInfo(qb);
- SimpleEntry<Integer,Integer> entry =
- qbp.getDestToLimit().get(qbp.getClauseNames().iterator().next());
- Integer offset = (entry == null) ? null : entry.getKey();
- Integer fetch = (entry == null) ? null : entry.getValue();
-
- if (fetch != null) {
- RexNode offsetRN = (offset == null || offset == 0) ?
- null : cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(offset));
- RexNode fetchRN = cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(fetch));
- RelTraitSet traitSet = cluster.traitSetOf(HiveRelNode.CONVENTION);
- RelCollation canonizedCollation = traitSet.canonize(RelCollations.EMPTY);
- sortRel = new HiveSortLimit(cluster, traitSet, srcRel, canonizedCollation, offsetRN, fetchRN);
-
- RowResolver inputRR = relToHiveRR.get(srcRel);
- RowResolver outputRR = inputRR.duplicate();
- ImmutableMap<String, Integer> hiveColNameCalcitePosMap = buildHiveToCalciteColumnMap(outputRR);
- relToHiveRR.put(sortRel, outputRR);
- relToHiveColNameCalcitePosMap.put(sortRel, hiveColNameCalcitePosMap);
+ /**
+ * Generates values from constant expression node.
+ * @param inputRR Row resolver
+ * @param expr AST Node which is a constant expression
+ * @return Object value of the expression
+ * @throws SemanticException
+ */
+ private Object genValueFromConstantExpr(RowResolver inputRR, ASTNode expr) throws SemanticException {
+ ExprNodeDesc exprNode = genExprNodeDesc(expr, inputRR, true, true);
+ if (exprNode instanceof ExprNodeConstantDesc) {
+ ExprNodeConstantDesc offsetConstantExprNode = (ExprNodeConstantDesc) exprNode;
+ return offsetConstantExprNode.getValue();
+ } else {
+ throw new SemanticException("Only constant expressions are supported");
}
+ }
- return sortRel;
+ private RelNode genLimitLogicalPlan(QB qb, RelNode srcRel, RelCollation canonizedCollation) throws SemanticException {
+ QBParseInfo qbp = getQBParseInfo(qb);
+ RowResolver inputRR = relToHiveRR.get(srcRel);
+ // Fetch the limit value from the query, or generate it from the limit expression if it exists
+ Integer limitValue = qbp.getDestLimit(qbp.getClauseNames().iterator().next());
Review Comment:
Maybe store qbp.getClauseNames().iterator().next() in a local variable and reuse - it might make the code a tiny more readable.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -4179,30 +4170,53 @@ public RelNode endGenOBLogicalPlan(OBLogicalPlanGenState obLogicalPlanGenState,
}
}
- private RelNode genLimitLogicalPlan(QB qb, RelNode srcRel) throws SemanticException {
- HiveRelNode sortRel = null;
- QBParseInfo qbp = getQBParseInfo(qb);
- SimpleEntry<Integer,Integer> entry =
- qbp.getDestToLimit().get(qbp.getClauseNames().iterator().next());
- Integer offset = (entry == null) ? null : entry.getKey();
- Integer fetch = (entry == null) ? null : entry.getValue();
-
- if (fetch != null) {
- RexNode offsetRN = (offset == null || offset == 0) ?
- null : cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(offset));
- RexNode fetchRN = cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(fetch));
- RelTraitSet traitSet = cluster.traitSetOf(HiveRelNode.CONVENTION);
- RelCollation canonizedCollation = traitSet.canonize(RelCollations.EMPTY);
- sortRel = new HiveSortLimit(cluster, traitSet, srcRel, canonizedCollation, offsetRN, fetchRN);
-
- RowResolver inputRR = relToHiveRR.get(srcRel);
- RowResolver outputRR = inputRR.duplicate();
- ImmutableMap<String, Integer> hiveColNameCalcitePosMap = buildHiveToCalciteColumnMap(outputRR);
- relToHiveRR.put(sortRel, outputRR);
- relToHiveColNameCalcitePosMap.put(sortRel, hiveColNameCalcitePosMap);
+ /**
+ * Generates values from constant expression node.
+ * @param inputRR Row resolver
+ * @param expr AST Node which is a constant expression
+ * @return Object value of the expression
+ * @throws SemanticException
+ */
+ private Object genValueFromConstantExpr(RowResolver inputRR, ASTNode expr) throws SemanticException {
+ ExprNodeDesc exprNode = genExprNodeDesc(expr, inputRR, true, true);
+ if (exprNode instanceof ExprNodeConstantDesc) {
+ ExprNodeConstantDesc offsetConstantExprNode = (ExprNodeConstantDesc) exprNode;
+ return offsetConstantExprNode.getValue();
+ } else {
+ throw new SemanticException("Only constant expressions are supported");
Review Comment:
Might be nice if you specify where constant expressions are expected - I.E. OFFSET LIMIT?
It would help the user determine where to look for the error in the query.
Are you able to write a query to trigger this exception? If so, I would consider adding a negative test.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -4179,30 +4170,53 @@ public RelNode endGenOBLogicalPlan(OBLogicalPlanGenState obLogicalPlanGenState,
}
}
- private RelNode genLimitLogicalPlan(QB qb, RelNode srcRel) throws SemanticException {
- HiveRelNode sortRel = null;
- QBParseInfo qbp = getQBParseInfo(qb);
- SimpleEntry<Integer,Integer> entry =
- qbp.getDestToLimit().get(qbp.getClauseNames().iterator().next());
- Integer offset = (entry == null) ? null : entry.getKey();
- Integer fetch = (entry == null) ? null : entry.getValue();
-
- if (fetch != null) {
- RexNode offsetRN = (offset == null || offset == 0) ?
- null : cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(offset));
- RexNode fetchRN = cluster.getRexBuilder().makeExactLiteral(BigDecimal.valueOf(fetch));
- RelTraitSet traitSet = cluster.traitSetOf(HiveRelNode.CONVENTION);
- RelCollation canonizedCollation = traitSet.canonize(RelCollations.EMPTY);
- sortRel = new HiveSortLimit(cluster, traitSet, srcRel, canonizedCollation, offsetRN, fetchRN);
-
- RowResolver inputRR = relToHiveRR.get(srcRel);
- RowResolver outputRR = inputRR.duplicate();
- ImmutableMap<String, Integer> hiveColNameCalcitePosMap = buildHiveToCalciteColumnMap(outputRR);
- relToHiveRR.put(sortRel, outputRR);
- relToHiveColNameCalcitePosMap.put(sortRel, hiveColNameCalcitePosMap);
+ /**
+ * Generates values from constant expression node.
+ * @param inputRR Row resolver
+ * @param expr AST Node which is a constant expression
+ * @return Object value of the expression
+ * @throws SemanticException
+ */
+ private Object genValueFromConstantExpr(RowResolver inputRR, ASTNode expr) throws SemanticException {
+ ExprNodeDesc exprNode = genExprNodeDesc(expr, inputRR, true, true);
+ if (exprNode instanceof ExprNodeConstantDesc) {
+ ExprNodeConstantDesc offsetConstantExprNode = (ExprNodeConstantDesc) exprNode;
+ return offsetConstantExprNode.getValue();
+ } else {
+ throw new SemanticException("Only constant expressions are supported");
}
+ }
- return sortRel;
+ private RelNode genLimitLogicalPlan(QB qb, RelNode srcRel, RelCollation canonizedCollation) throws SemanticException {
+ QBParseInfo qbp = getQBParseInfo(qb);
+ RowResolver inputRR = relToHiveRR.get(srcRel);
+ // Fetch the limit value from the query, or generate it from the limit expression if it exists
+ Integer limitValue = qbp.getDestLimit(qbp.getClauseNames().iterator().next());
+ ASTNode limitExpr = qbp.getDestASTLimit(qbp.getClauseNames().iterator().next());
+ if (limitValue == null && limitExpr != null) {
+ limitValue = (Integer) genValueFromConstantExpr(inputRR, limitExpr);
+ }
+ // Fetch the offset value from the query, or generate it from the offset expression if it exists
+ Integer offsetValue = qbp.getDestLimitOffset(qbp.getClauseNames().iterator().next());
+ ASTNode offsetExpr = qbp.getDestASTOffset(qbp.getClauseNames().iterator().next());
+ if (offsetValue == null && offsetExpr != null) {
+ offsetValue = (Integer) genValueFromConstantExpr(inputRR, offsetExpr);
+ }
+ if (limitValue == null) {
Review Comment:
This likely should moved to just after limitValue is set
Issue Time Tracking
-------------------
Worklog Id: (was: 853965)
Remaining Estimate: 0h
Time Spent: 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
> Time Spent: 10m
> 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)