[GitHub] [calcite] amaliujia commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…

2020-06-24 Thread GitBox


amaliujia commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445213183



##
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:
   Added a test to test null values. Based on the test, null is put at the 
last position (ASC) by comparator.





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] amaliujia commented on a change in pull request #2035: [CALCITE-4008] Implement Code generation for EnumerableSortedAggregat…

2020-06-24 Thread GitBox


amaliujia commented on a change in pull request #2035:
URL: https://github.com/apache/calcite/pull/2035#discussion_r445212926



##
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:
   Thanks for the example!
   
   I doubled check the contract and have the following conclusion:
   ```
  * After an enumerator is created or after the {@link #reset} method is
  * called, the {@link #moveNext} method must be called to advance the
  * enumerator to the first element of the collection before reading the
  * value of the {@code current} property; otherwise, {@code current} is
  * undefined.
   ```
   So in this case, return `null` is right as the result is undefined.
   
   ```
  * This method also throws {@link java.util.NoSuchElementException} if
  * the last call to {@code moveNext} returned {@code false}, which 
indicates
  * the end of the collection.
   ```
   I updated the code to make sure it throws `NoSuchElementException` in this 
case.
   
   





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




[calcite] branch master updated: [CALCITE-4087] Hoist, a utility to replace literals in a SQL string with placeholders

2020-06-24 Thread jhyde
This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new 6f90aca  [CALCITE-4087] Hoist, a utility to replace literals in a SQL 
string with placeholders
6f90aca is described below

commit 6f90acaaac962f666741dae8fa20170e1d9a71e4
Author: Julian Hyde 
AuthorDate: Tue Jun 23 17:06:50 2020 -0700

[CALCITE-4087] Hoist, a utility to replace literals in a SQL string with 
placeholders
---
 .../org/apache/calcite/test/BabelParserTest.java   |  39 +
 .../main/java/org/apache/calcite/tools/Hoist.java  | 191 +
 .../apache/calcite/sql/parser/SqlParserTest.java   |  51 ++
 3 files changed, 281 insertions(+)

diff --git a/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java 
b/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
index b5d185f..47d5500 100644
--- a/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
+++ b/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
@@ -16,11 +16,15 @@
  */
 package org.apache.calcite.test;
 
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.calcite.sql.dialect.MysqlSqlDialect;
 import org.apache.calcite.sql.parser.SqlAbstractParserImpl;
+import org.apache.calcite.sql.parser.SqlParser;
 import org.apache.calcite.sql.parser.SqlParserImplFactory;
 import org.apache.calcite.sql.parser.SqlParserTest;
 import org.apache.calcite.sql.parser.SqlParserUtil;
 import org.apache.calcite.sql.parser.babel.SqlBabelParserImpl;
+import org.apache.calcite.tools.Hoist;
 
 import com.google.common.base.Throwables;
 
@@ -281,4 +285,39 @@ class BabelParserTest extends SqlParserTest {
 + "(`BAR` INTEGER NOT NULL, `BAZ` VARCHAR(30))";
 sql(sql).ok(expected);
   }
+
+  /** Similar to {@link #testHoist()} but using custom parser. */
+  @Test void testHoistMySql() {
+// SQL contains back-ticks, which require MySQL's quoting,
+// and DATEADD, which requires Babel.
+final String sql = "select 1 as x,\n"
++ "  'ab' || 'c' as y\n"
++ "from `my emp` /* comment with 'quoted string'? */ as e\n"
++ "where deptno < 40\n"
++ "and DATEADD(day, 1, hiredate) > date '2010-05-06'";
+final SqlDialect dialect = MysqlSqlDialect.DEFAULT;
+final Hoist.Hoisted hoisted =
+Hoist.create(Hoist.config()
+.withParserConfig(
+dialect.configureParser(SqlParser.configBuilder())
+.setParserFactory(SqlBabelParserImpl::new)
+.build()))
+.hoist(sql);
+
+// Simple toString converts each variable to '?N'
+final String expected = "select ?0 as x,\n"
++ "  ?1 || ?2 as y\n"
++ "from `my emp` /* comment with 'quoted string'? */ as e\n"
++ "where deptno < ?3\n"
++ "and DATEADD(day, ?4, hiredate) > ?5";
+assertThat(hoisted.toString(), is(expected));
+
+// Custom string converts variables to '[N:TYPE:VALUE]'
+final String expected2 = "select [0:DECIMAL:1] as x,\n"
++ "  [1:CHAR:ab] || [2:CHAR:c] as y\n"
++ "from `my emp` /* comment with 'quoted string'? */ as e\n"
++ "where deptno < [3:DECIMAL:40]\n"
++ "and DATEADD(day, [4:DECIMAL:1], hiredate) > [5:DATE:2010-05-06]";
+assertThat(hoisted.substitute(SqlParserTest::varToStr), is(expected2));
+  }
 }
diff --git a/core/src/main/java/org/apache/calcite/tools/Hoist.java 
b/core/src/main/java/org/apache/calcite/tools/Hoist.java
new file mode 100644
index 000..c8e7f21
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/tools/Hoist.java
@@ -0,0 +1,191 @@
+/*
+ * 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.tools;
+
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.parser.SqlParseException;
+import org.apache.calcite.sql.parser.SqlParser;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.parser.SqlParserUtil;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.util.SqlShuttle;

[calcite] branch master updated: [CALCITE-3786] Make digestEquals and digestHash available to be overridden

2020-06-24 Thread hyuan
This is an automated email from the ASF dual-hosted git repository.

hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new af3bca3  [CALCITE-3786] Make digestEquals and digestHash available to 
be overridden
af3bca3 is described below

commit af3bca328a40d2d6515ea00e2094974cc725d4c3
Author: Haisheng Yuan 
AuthorDate: Tue Jun 23 17:20:37 2020 -0500

[CALCITE-3786] Make digestEquals and digestHash available to be overridden

By default #digestEquals() and #digestHash() collect digest attributes from
compute hash code. This should work well for most cases. If the method is a
performance bottleneck for your project, or the default behavior can't 
handle
your scenario properly, you can choose to override #digestEquals() and
Only these operators are changed to override the default behavior, for
performance and demonstration purposes. All the other operators remain
unchanged.

Close #2044
---
 .../java/org/apache/calcite/plan/RelOptNode.java   | 15 +--
 .../org/apache/calcite/plan/hep/HepRelVertex.java  | 11 +---
 .../org/apache/calcite/rel/AbstractRelNode.java| 26 --
 .../main/java/org/apache/calcite/rel/RelNode.java  | 24 ++---
 .../java/org/apache/calcite/rel/core/Filter.java   | 19 +
 .../java/org/apache/calcite/rel/core/Join.java | 21 +++
 .../java/org/apache/calcite/rel/core/Project.java  | 20 ++
 .../apache/calcite/rel/logical/LogicalFilter.java  |  9 +++
 .../apache/calcite/rel/logical/LogicalJoin.java| 13 +
 .../apache/calcite/rel/logical/LogicalProject.java |  8 ++
 .../org/apache/calcite/rel/type/RelDataType.java   | 31 ++
 11 files changed, 169 insertions(+), 28 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptNode.java 
b/core/src/main/java/org/apache/calcite/plan/RelOptNode.java
index 208a87c..c21f745 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptNode.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptNode.java
@@ -18,8 +18,6 @@ package org.apache.calcite.plan;
 
 import org.apache.calcite.rel.type.RelDataType;
 
-import org.apiguardian.api.API;
-
 import java.util.List;
 
 /**
@@ -49,18 +47,7 @@ public interface RelOptNode {
*
* @return Digest string of this {@code RelNode}
*/
-  default String getDigest() {
-return getRelDigest().toString();
-  }
-
-  /**
-   * Digest of the {@code RelNode}, for planner internal use only.
-   *
-   * @return Digest of this {@code RelNode}
-   * @see #getDigest()
-   */
-  @API(since = "1.24", status = API.Status.INTERNAL)
-  RelDigest getRelDigest();
+  String getDigest();
 
   /**
* Retrieves this RelNode's traits. Note that although the RelTraitSet
diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java 
b/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java
index 1061a61..4ad6d28 100644
--- a/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java
+++ b/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java
@@ -91,9 +91,14 @@ public class HepRelVertex extends AbstractRelNode {
 return currentRel;
   }
 
-  @Override public RelWriter explainTerms(final RelWriter pw) {
-return super.explainTerms(pw)
-.item("currentRel", currentRel.getId());
+  @Override protected boolean digestEquals(Object obj) {
+return this == obj
+|| (obj instanceof HepRelVertex
+&& currentRel == ((HepRelVertex) obj).currentRel);
+  }
+
+  @Override protected int digestHash() {
+return currentRel.getId();
   }
 
   @Override public String getDigest() {
diff --git a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java 
b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
index 4ab68a0..ec52c10 100644
--- a/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
+++ b/core/src/main/java/org/apache/calcite/rel/AbstractRelNode.java
@@ -337,9 +337,8 @@ public abstract class AbstractRelNode implements RelNode {
 return r;
   }
 
-  public RelDigest recomputeDigest() {
+  public void recomputeDigest() {
 digest.clear();
-return digest;
   }
 
   public void replaceInput(
@@ -394,9 +393,19 @@ public abstract class AbstractRelNode implements RelNode {
   }
 
   /**
-   * Equality check for RelNode digest
+   * Equality check for RelNode digest.
+   *
+   * By default this method collects digest attributes from
+   * {@link #explainTerms(RelWriter)}, then compares each attribute pair.
+   * This should work well for most cases. If this method is a performance
+   * bottleneck for your project, or the default behavior can't handle
+   * your scenario properly, you can choose to override this method and
+   * {@link #digestHash()}. See {@code LogicalJoin} as an example.
+   *
+   * @return Whether the 2 

[GitHub] [calcite] hsyuan merged pull request #2044: [CALCITE-3786] Make digestEquals and digestHash available to be overridden

2020-06-24 Thread GitBox


hsyuan merged pull request #2044:
URL: https://github.com/apache/calcite/pull/2044


   



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




[calcite] branch master updated: [CALCITE-4086] Upgrade Avatica version to 1.17.0

2020-06-24 Thread zabetak
This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
 new b0b435c  [CALCITE-4086] Upgrade Avatica version to 1.17.0
b0b435c is described below

commit b0b435cae05d2d96326e3f7165bfaea4c2b28e52
Author: Stamatis Zampetakis 
AuthorDate: Wed Jun 24 00:40:11 2020 +0200

[CALCITE-4086] Upgrade Avatica version to 1.17.0
---
 gradle.properties | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gradle.properties b/gradle.properties
index ea5b9aa..d67b6a2 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -25,7 +25,7 @@ kotlin.parallel.tasks.in.project=true
 # Release version can be generated by using -Prelease or -Prc= arguments
 calcite.version=1.24.0
 # This is a version to be used from Maven repository. It can be overridden by 
localAvatica below
-calcite.avatica.version=1.16.0
+calcite.avatica.version=1.17.0
 
 # The options below configures the use of local clone (e.g. testing 
development versions)
 # You can pass un-comment it, or pass option -PlocalReleasePlugins, or 
-PlocalReleasePlugins=



[GitHub] [calcite] zabetak merged pull request #2043: [CALCITE-4086] Upgrade Avatica version to 1.17.0

2020-06-24 Thread GitBox


zabetak merged pull request #2043:
URL: https://github.com/apache/calcite/pull/2043


   



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…

2020-06-24 Thread GitBox


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…

2020-06-24 Thread GitBox


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