Vladsz83 commented on code in PR #13407:
URL: https://github.com/apache/ignite/pull/13407#discussion_r3675118877
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/SortAggregateNode.java:
##########
@@ -101,8 +101,11 @@ public SortAggregateNode(
source().request(IN_BUFFER_SIZE);
}
- else if (waiting < 0)
+ else if (waiting < 0) {
Review Comment:
Same. Looks like a delayed `end()`. Why not in own `end()`. It it some `CLN
node` behaviour trait?
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/SortNode.java:
##########
@@ -205,10 +205,11 @@ private void flush() throws Exception {
}
if (reversed == null ? rows.isEmpty() : reversed.isEmpty()) {
- if (requested > 0)
- downstream().end();
+ if (requested > 0) {
Review Comment:
The new tests `testRequestRowsAfterInputEnds()` and `CollectIntegrationTest`
seem to work witout this change. Why? Maybe we should add/fix some executions
tests too.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/SortAggregateNode.java:
##########
Review Comment:
If we do conditional `requested = 0;`, probably we should not assignt it here
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/CorrelatedNestedLoopJoinNode.java:
##########
@@ -282,6 +282,8 @@ private void onRequest() throws Exception {
break;
case END:
+ requested = 0;
Review Comment:
New `testRequestRowsAfterInputEnds()` test seems to work without any changes
here. Is it an optimization? Do we need extra execution tests or at least
comments?
--
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]
