alex-plekhanov commented on code in PR #13359:
URL: https://github.com/apache/ignite/pull/13359#discussion_r4104985539
##########
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:
Redundant NL
##########
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:
First letter - uppercase
##########
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:
Looks like this method can be boolean (always checked for != 0, maybe use it
as `isReady`), up to you.
##########
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,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
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,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
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,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/WindowExecutionTest.java:
##########
@@ -550,7 +572,8 @@ private void checkWindow(
}
/** */
- private WindowNode<Object[]> createWindowNode(ExecutionContext<Object[]>
ctx, Window.Group grp, Node<Object[]> input) {
+ private Node<Object[]> createWindowNode(ExecutionContext<Object[]> ctx,
Window.Group grp,
Review Comment:
Codestyle: each parameter should be on it's own line.
--
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]
