mihaibudiu commented on code in PR #5293:
URL: https://github.com/apache/calcite/pull/5293#discussion_r4170427585
##########
core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -2623,26 +2623,27 @@ private RexNode convertOver(Blackboard bb, SqlNode
node) {
bb.convertExpression(requireNonNull(sqlLowerBound, "sqlLowerBound"));
final RexNode upperBound =
bb.convertExpression(requireNonNull(sqlUpperBound, "sqlUpperBound"));
- if (orderList.isEmpty() && !rows) {
- // A logical range requires an ORDER BY clause. Use the implicit
- // ordering of this relation. There must be one, otherwise it would
- // have failed validation.
- orderList = bb.scope.getOrderList();
- if (orderList == null) {
- throw new AssertionError(
- "Relation should have sort key for implicit ORDER BY");
- }
- }
final RexWindowExclusion exclude =
RexWindowExclusion.create(window.getExclude());
final ImmutableList.Builder<RexNode> orderKeys =
ImmutableList.builder();
- for (SqlNode order : orderList) {
- orderKeys.add(
- bb.convertSortExpression(order,
- RelFieldCollation.Direction.ASCENDING,
- RelFieldCollation.NullDirection.UNSPECIFIED,
- bb::sortToRex));
+ if (orderList.isEmpty() && !rows) {
Review Comment:
I thought we agreed to leave this part unchanged.
The fix should have nothing to do with the implicit order.
--
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]
