Akshat-Jain commented on code in PR #17038:
URL: https://github.com/apache/druid/pull/17038#discussion_r1774546394
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/WindowOperatorQueryFrameProcessor.java:
##########
@@ -338,6 +218,9 @@ private void flushAllRowsAndCols(ArrayList<RowsAndColumns>
resultRowAndCols) thr
AtomicInteger rowId = new AtomicInteger(0);
createFrameWriterIfNeeded(rac, rowId);
writeRacToFrame(rac, rowId);
Review Comment:
> if this method would be called 2 times the 1st invocation of
createFrameWriterIfNeeded would still retain the previous rowid reference
```java
AtomicInteger rowId = new AtomicInteger(0);
createFrameWriterIfNeeded(rac, rowId);
writeRacToFrame(rac, rowId);
```
In the above lines, `writeRacToFrame()` calls `flushFrameWriter()`, which
has the following:
```java
frameWriter.close();
frameWriter = null;
```
So each invocation of `createFrameWriterIfNeeded()` ends up creating a new
frame writer IIUC.
Would doing the following 2 things help?
1. Remove `if (frameWriter == null)` condition from
`createFrameWriterIfNeeded()` method as it always seems to be true? 🤔
2. Move `frameWriter.close(); frameWriter = null;` into a different method
`closeFrameWriter()`, remove them from `flushFrameWriter()`, and call
`closeFrameWriter()` after `writeRacToFrame()`?
Thoughts?
--
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]
