oleg-zinovev commented on code in PR #13359:
URL: https://github.com/apache/ignite/pull/13359#discussion_r4182682732


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/window/BufferingWindowPartition.java:
##########
@@ -48,60 +57,64 @@ final class BufferingWindowPartition<Row> extends 
WindowPartitionBase<Row> {
         RelDataType inputRowType
     ) {
         super(peerCmp, funcFactory, rowFactory);
-        buf = new ArrayList<>();
-        frame = createFrame(ctx, grp, peerCmp, inputRowType, buf);
-    }
-
-    /** {@inheritDoc} */
-    @Override public void add(Row row) {
-        buf.add(row);
-        onRowAdded(row);
+        this.ctx = ctx;
+        this.grp = grp;
+        this.inputRowType = inputRowType;
+        frames = new ArrayDeque<>();
     }
 
-    /** {@inheritDoc} */
-    @Override public void evalTo(RowHandler.RowFactory<Row> factory, 
Consumer<Row> output) {
+    /**
+     * Appends rows to the partition.
+     * Important: the buffer must not be modified after being passed to this 
method.
+     **/
+    public void appendPartition(List<Row> buf, Runnable onBufRemoved) {
         if (buf.isEmpty())
             return;
 
-        List<WindowFunctionWrapper<Row>> accumulators = createWrappers();
-        Object[] accResults = new Object[accumulators.size()];
+        WindowPartitionFrame<Row> frame = createFrame(ctx, grp, peerCmp, 
inputRowType, buf);
+        ready += frame.size();
+        frames.add(new FrameHolder(frame, onBufRemoved));
+    }
 
-        int size = buf.size();
-        Row prevRow = null;
-        int peerIdx = -1;
-        for (int rowIdx = 0; rowIdx < size; rowIdx++) {
-            Row currRow = buf.get(rowIdx);
-            if (isNewPeer(currRow, prevRow))
-                peerIdx++;
+    /**
+     * Evaluates next row in the window partition.
+     * @return Result row or {@code null} if there are no more rows.
+     */
+    public @Nullable Row nextRow(RowHandler.RowFactory<Row> factory) {
+        if (currFrame == null || currFrame.consumed()) {
+            if (currFrame != null)
+                currFrame.release();
+            currFrame = frames.pollFirst();
+        }
 
-            int accIdx = 0;
-            for (WindowFunctionWrapper<Row> acc : accumulators) {
-                Object accResult = acc.callBuffering(currRow, rowIdx, peerIdx, 
frame);
-                accResults[accIdx++] = accResult;
-            }
+        if (currFrame == null) {
+            assert ready == 0;
+            return null;
+        }
 
-            Row resultRow = createResultRow(factory, currRow, accResults);
-            output.accept(resultRow);
+        assert ready > 0;
+        Row resultRow = currFrame.nextRow(factory);
+        ready--;
+        return resultRow;
+    }
 
-            prevRow = currRow;
-        }
+    /** Returns the number of rows can be evaluted. */
+    public int ready() {

Review Comment:
   renamed to isReady



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/BufferingWindowNode.java:
##########
@@ -124,9 +134,11 @@ else if (prevRow != null && partCmp != null && 
partCmp.compare(prevRow, row) !=
 
         checkState();
 
-        if (part != null) {
-            part.evalTo(rowFactory, this::appendToOutBuf);
-            part.reset();
+        // append partition for remaining rows if any.

Review Comment:
   fixed



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/window/WindowPartitionFactory.java:
##########
@@ -28,56 +27,55 @@
 import org.apache.ignite.internal.processors.query.calcite.util.Commons;
 
 /** Factory to create {@link WindowPartitionBase} factory from {@link 
Window.Group}. */
-public final class WindowPartitionFactory<Row> implements 
Supplier<WindowPartition<Row>> {
-    /** */
-    private final WindowFunctionFactory<Row> funcFactory;
-
+public final class WindowPartitionFactory<Row> {
     /** */
     private final ExecutionContext<Row> ctx;
 
     /** */
-    private final Window.Group grp;
+    public WindowPartitionFactory(ExecutionContext<Row> ctx) {
+        this.ctx = ctx;
+    }
 
-    /** */
-    private final RelDataType inputRowType;
+    /** Creates streaming window partition for provieded group, aggragates and 
input row type, */
+    public StreamingWindowPartition<Row> newStreamingPartition(Window.Group 
grp, List<AggregateCall> calls,
+        RelDataType inputRowType) {
 
-    /** */
-    private final RowHandler.RowFactory<Row> rowFactory;
+        assert WindowFunctions.streamable(grp);
 
-    /** */
-    private final Comparator<Row> peerCmp;
+        return createPartition(grp, calls, inputRowType, 
StreamingWindowPartition::new);
+    }
 
-    /** */
-    private final boolean streamable;
+    /** Creates buffering window partition for provieded group, aggragates and 
input row type, */
+    public BufferingWindowPartition<Row> newBufferingPartition(Window.Group 
grp, List<AggregateCall> calls,
+        RelDataType inputRowType) {
 
-    /** */
-    public WindowPartitionFactory(
-        ExecutionContext<Row> ctx,
-        Window.Group grp,
-        List<AggregateCall> calls,
-        RelDataType inputRowType
-    ) {
-        this.ctx = ctx;
-        this.grp = grp;
-        this.inputRowType = inputRowType;
+        return createPartition(grp, calls, inputRowType, (peerCmp, 
funcFactory, rowFactory)
+            -> new BufferingWindowPartition<>(peerCmp, funcFactory, 
rowFactory, ctx, grp, inputRowType));
+    }
 
-        List<RelDataType> aggTypes = Commons.transform(calls, 
AggregateCall::getType);
-        rowFactory = ctx.rowHandler().factory(Commons.typeFactory(), aggTypes);
+    /** */
+    private <T extends WindowPartition<Row>> T createPartition(Window.Group 
grp, List<AggregateCall> calls,
+        RelDataType inputRowType, PartitionCreator<Row, T> creator) {
+        Comparator<Row> peerCmp;
         if (grp.isRows)
             // peer comparator in meaningless in rows frame.
             peerCmp = null;
         else
             peerCmp = ctx.expressionFactory().comparator(grp.collation());
 
-        streamable = WindowFunctions.streamable(grp);
-        funcFactory = new WindowFunctionFactory<>(ctx, grp, calls, 
inputRowType);
+        WindowFunctionFactory<Row> funcFactory = new 
WindowFunctionFactory<>(ctx, grp, calls, inputRowType);
+
+        List<RelDataType> aggTypes = Commons.transform(calls, 
AggregateCall::getType);
+        RowHandler.RowFactory<Row> rowFactory = 
ctx.rowHandler().factory(Commons.typeFactory(), aggTypes);
+
+        return creator.create(peerCmp, funcFactory, rowFactory);
     }
 
-    /** {@inheritDoc} */
-    @Override public WindowPartition<Row> get() {
-        if (streamable)
-            return new StreamWindowPartition<>(peerCmp, funcFactory, 
rowFactory);
-        else
-            return new BufferingWindowPartition<>(peerCmp, funcFactory, 
rowFactory, ctx, grp, inputRowType);
+    /** */
+    @FunctionalInterface
+    private interface PartitionCreator<Row, T extends WindowPartition<?>> {
+        /** */
+        T create(Comparator<Row> peerCmp, WindowFunctionFactory<Row> 
funcFactory, RowHandler.RowFactory<Row> rowFactory);
     }
+

Review Comment:
   fixed



-- 
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]

Reply via email to