[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445351713
##
File path:
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableSortedAggregateTest.java
##
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test.enumerable;
+
+import org.apache.calcite.adapter.enumerable.EnumerableRules;
+import org.apache.calcite.adapter.java.ReflectiveSchema;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.config.Lex;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.runtime.Hook;
+import org.apache.calcite.test.CalciteAssert;
+import org.apache.calcite.test.JdbcTest;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.function.Consumer;
+
+public class EnumerableSortedAggregateTest {
+ @Test void sortedAgg() {
Review comment:
I would propose e.g.
- group by X, Y (several columns)
- group by X + order by X
- group by X + order by Y
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445352328
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,132 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private boolean isLastMoveNextFalse;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+private TResult curResult;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+ curResult = null;
+ isLastMoveNextFalse = false;
+}
+
+@Override public TResult current() {
+ if (isLastMoveNextFalse) {
+throw new NoSuchElementException();
+ }
+ return curResult;
+}
+
+@Override public boolean moveNext() {
+ if (!isInitialized) {
+isInitialized = true;
+// input is empty
+if (!enumerator.moveNext()) {
+ isLastMoveNextFalse = true;
+ return false;
+}
+ } else if (isInitialized && curAccumulator == null) {
+// input has been exhausted.
+isLastMoveNextFalse = true;
+return false;
+ }
+
+ if (curAccumulator == null) {
+curAccumulator = accumulatorInitializer.apply();
+ }
+
+ // reset result because now it can move to next aggregated result.
+ curResult = null;
+ TSource o = enumerator.current();
+ TKey prevKey = keySelector.apply(o);
+ curAccumulator = accumulatorAdder.apply(curAccumulator, o);
+ while (enumerator.moveNext()) {
+o = enumerator.current();
+TKey curKey = keySelector.apply(o);
+if (comparator.compare(prevKey, curKey) != 0) {
+ // current key is different from previous key, get accumulated
results and re-create
+ // accumulator for current key.
+ curResult = resultSelector.apply(prevKey, curAccumulator);
+ curAccumulator = accumulatorInitializer.apply();
+ break;
+} else {
Review comment:
I think we can remove the `else`, because there is a `break` at the end
of the `if` block. IMO that would meake the code clearer.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445353311
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,132 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private boolean isLastMoveNextFalse;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+private TResult curResult;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+ curResult = null;
+ isLastMoveNextFalse = false;
+}
+
+@Override public TResult current() {
+ if (isLastMoveNextFalse) {
+throw new NoSuchElementException();
+ }
+ return curResult;
+}
+
+@Override public boolean moveNext() {
+ if (!isInitialized) {
+isInitialized = true;
+// input is empty
+if (!enumerator.moveNext()) {
+ isLastMoveNextFalse = true;
+ return false;
+}
+ } else if (isInitialized && curAccumulator == null) {
Review comment:
I think checking `isInitialized` in this line is redundant. At this
point `isInitialized` will always be true (otherwise we would have went into
the first `if` block.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445353311
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,132 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private boolean isLastMoveNextFalse;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+private TResult curResult;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+ curResult = null;
+ isLastMoveNextFalse = false;
+}
+
+@Override public TResult current() {
+ if (isLastMoveNextFalse) {
+throw new NoSuchElementException();
+ }
+ return curResult;
+}
+
+@Override public boolean moveNext() {
+ if (!isInitialized) {
+isInitialized = true;
+// input is empty
+if (!enumerator.moveNext()) {
+ isLastMoveNextFalse = true;
+ return false;
+}
+ } else if (isInitialized && curAccumulator == null) {
Review comment:
I think checking `isInitialized` in this line is redundant. At this
point `isInitialized` will always be true (otherwise we would have went into
the first `if` block).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445349359
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
+ if (curAccumulator == null) {
+curAccumulator = accumulatorInitializer.apply();
+ }
+ TResult result = null;
+ TSource o = enumerator.current();
+ TKey prevKey = keySelector.apply(o);
+ curAccumulator = accumulatorAdder.apply(curAccumulator, o);
+ while (enumerator.moveNext()) {
+o = enumerator.current();
+TKey curKey = keySelector.apply(o);
Review comment:
Thanks for adding test, good example.
Maybe it might be worth it adding a line in `sortedGroupBy` javadocs
specifying that _in case of null keys, the comparator must be able to support
null values_ (or something along these lines).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445347641
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
Review comment:
You're right. Re-reading the javadoc of `current` and its `throws`
clauses, I find it a bit confusing, even contradictory, regarding this type of
situations. In any case, this is out of the scope of the current PR, so IMO
your `current` proposal is ok.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r444744863
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
Review comment:
Also a minor detail, please notice that, per contract, `current` must
throw `NoSuchElementException` if `moveNext` has not been called, in your case
I think it would just return `null`.
You can easily fix this by using a DUMMY object, see e.g.
`lazyCollectionSpool` method
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r444738980
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
+ if (curAccumulator == null) {
+curAccumulator = accumulatorInitializer.apply();
+ }
+ TResult result = null;
+ TSource o = enumerator.current();
+ TKey prevKey = keySelector.apply(o);
+ curAccumulator = accumulatorAdder.apply(curAccumulator, o);
+ while (enumerator.moveNext()) {
+o = enumerator.current();
+TKey curKey = keySelector.apply(o);
Review comment:
NULLs should always come at the beginning or at the end, so I assume the
current algorithm would work... if the comparator is able to support nulls.
Otherwise, as you say, we would need to add an extra check for null values.
I guess this will be an good scenario for a unit test
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r443486637
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
+ if (curAccumulator == null) {
+curAccumulator = accumulatorInitializer.apply();
+ }
+ TResult result = null;
+ TSource o = enumerator.current();
+ TKey prevKey = keySelector.apply(o);
+ curAccumulator = accumulatorAdder.apply(curAccumulator, o);
+ while (enumerator.moveNext()) {
+o = enumerator.current();
+TKey curKey = keySelector.apply(o);
+if (comparator.compare(prevKey, curKey) != 0) {
+ // current key is different from previous key, get accumulated
results and re-create
+ // accumulator for current key.
+ result = resultSelector.apply(prevKey, curAccumulator);
+ curAccumulator = accumulatorInitializer.apply();
+ break;
+} else {
+ curAccumulator = accumulatorAdder.apply(curAccumulator, o);
+}
+prevKey = curKey;
+ }
+
+ if (result == null) {
+// current key is the last key.
+result = resultSelector.apply(prevKey, curAccumulator);
+// no need to keep accumulator for the last key.
+curAccumulator = null;
+ }
+
+ return result;
+}
+
+@Override public boolean moveNext() {
+ if (!isInitialized) {
+isInitialized = true;
+return enumerator.moveNext();
+ } else {
+return curAccumulator != null;
+ }
+}
+
+@Override public void reset() {
+ enumerator.reset();
Review comment:
Maybe we should put `curAccumulator = null;` as part of the `reset`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r443451500
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
Review comment:
I think this implementation of `current` does not respect this part of
the contract:
```
This method does not move the position of the enumerator, and
consecutive calls to {@code current} return the same object until either
{@code moveNext} or {@code reset} is called.
```
I have the impression that consecutive calls to `current` will return
different results. I think the logic that we have here should be transferred as
part of `moveNext`, and the "current object" should be somehow saved; finally
`current` should do no processing, just returning the "current object" saved by
the `moveNext` process, or something like that.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r443446065
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -817,6 +817,112 @@ public void remove() {
resultSelector);
}
+ /**
+ * Group keys are sorted already. Key values are compared by using a
+ * specified comparator. Groups the elements of a sequence according to a
+ * specified key selector function and initializing one accumulator at a
time.
+ * Go over elements sequentially, adding to accumulator each time an element
+ * with the same key is seen. When key changes, creates a result value from
the
+ * accumulator and then re-initializes the accumulator.
+ */
+ public static Enumerable
sortedGroupBy(
+ Enumerable enumerable,
+ Function1 keySelector,
+ Function0 accumulatorInitializer,
+ Function2 accumulatorAdder,
+ final Function2 resultSelector,
+ final Comparator comparator) {
+return new AbstractEnumerable() {
+ public Enumerator enumerator() {
+return new SortedAggregateEnumerator(
+ enumerable, keySelector, accumulatorInitializer,
+ accumulatorAdder, resultSelector, comparator);
+ }
+};
+ }
+
+ private static class SortedAggregateEnumerator
+ implements Enumerator {
+private final Enumerable enumerable;
+private final Function1 keySelector;
+private final Function0 accumulatorInitializer;
+private final Function2
accumulatorAdder;
+private final Function2 resultSelector;
+private final Comparator comparator;
+private boolean isInitialized;
+private TAccumulate curAccumulator;
+private Enumerator enumerator;
+
+SortedAggregateEnumerator(
+Enumerable enumerable,
+Function1 keySelector,
+Function0 accumulatorInitializer,
+Function2 accumulatorAdder,
+final Function2 resultSelector,
+final Comparator comparator) {
+ this.enumerable = enumerable;
+ this.keySelector = keySelector;
+ this.accumulatorInitializer = accumulatorInitializer;
+ this.accumulatorAdder = accumulatorAdder;
+ this.resultSelector = resultSelector;
+ this.comparator = comparator;
+ isInitialized = false;
+ curAccumulator = null;
+ enumerator = enumerable.enumerator();
+}
+
+@Override public TResult current() {
+ if (curAccumulator == null) {
+curAccumulator = accumulatorInitializer.apply();
+ }
+ TResult result = null;
+ TSource o = enumerator.current();
+ TKey prevKey = keySelector.apply(o);
+ curAccumulator = accumulatorAdder.apply(curAccumulator, o);
+ while (enumerator.moveNext()) {
+o = enumerator.current();
+TKey curKey = keySelector.apply(o);
Review comment:
Would it be possible for a key to be `null`? Do we need to add a check
for that?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r443406186
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableSortedAggregate.java
##
@@ -90,6 +101,133 @@ public EnumerableSortedAggregate(
}
public Result implement(EnumerableRelImplementor implementor, Prefer pref) {
-throw Util.needToImplement("EnumerableSortedAggregate");
+if (getGroupType() != Group.SIMPLE
Review comment:
Would it be possible to extract these conditions as an auxiliary method
`isSupported` ?
By doing this, we could make EnumerableSortedAggregateRule to check it, and
in case of not supported, do not generate the EnumerableSortedAggregate
(otherwise the rule would generate an operator that will fail when trying to
implement).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…
rubenada commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r443406186
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableSortedAggregate.java
##
@@ -90,6 +101,133 @@ public EnumerableSortedAggregate(
}
public Result implement(EnumerableRelImplementor implementor, Prefer pref) {
-throw Util.needToImplement("EnumerableSortedAggregate");
+if (getGroupType() != Group.SIMPLE
Review comment:
Would it be possible to extract these conditions as an auxiliary method
`isSupported` ?
By doing this, we could make EnumerableSortedAggregateRule to check it, and
it case of not supported, do not generate the EnumerableSortedAggregate
(otherwise the rule would generate an operator that will fail when trying to
implement).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
