[ 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)