Copilot commented on code in PR #19823:
URL: https://github.com/apache/druid/pull/19823#discussion_r3720852036
##########
processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java:
##########
@@ -276,21 +276,15 @@ private Pair<byte[], RowSignature>
materializeCursorFactory(CursorFactory cursor
sortColumns
);
- final FrameWriter writer =
frameWriterFactory.newFrameWriter(columnSelectorFactory);
- for (; !cursor.isDoneOrInterrupted() && remainingRowsToSkip > 0;
remainingRowsToSkip--) {
- cursor.advance();
- }
- for (; !cursor.isDoneOrInterrupted() && remainingRowsToFetch > 0;
remainingRowsToFetch--) {
- writer.addSelection();
- cursor.advance();
- }
+ try (final FrameWriter writer =
frameWriterFactory.newFrameWriter(columnSelectorFactory)) {
+ for (; !cursor.isDoneOrInterrupted() && remainingRowsToSkip > 0;
remainingRowsToSkip--) {
+ cursor.advance();
+ }
+ for (; !cursor.isDoneOrInterrupted() && remainingRowsToFetch > 0;
remainingRowsToFetch--) {
+ writer.addSelection();
+ cursor.advance();
+ }
- if (writer == null) {
- // This means that the accumulate was never called, which can only
happen if we didn't have any cursors.
- // We would only have zero cursors if we essentially didn't match
anything, meaning that our RowsAndColumns
- // should be completely empty.
- return null;
- } else {
final byte[] bytes = writer.toByteArray();
return Pair.of(bytes, siggy.get());
}
Review Comment:
This change appears to remove the prior special-case where the method could
return `null` when no rows/cursors were materialized (previously described in
the removed `writer == null` comment). With try-with-resources, the method now
always returns a `Pair` (potentially representing an empty frame), which may
change expected semantics for callers that use `null` to represent “completely
empty / no cursors”. Consider preserving the old behavior explicitly (e.g.,
track whether any selections were added and return `null` if none) if
downstream code relies on `null` as a sentinel.
--
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]
