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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
