address review comments

h2/src/main/org/h2/command/dml/Select.java

@@ -187,9 +187,7 @@ public class Select extends Query {
     public void setCurrentGroupExprData(Expression expr, Object obj) {
         Integer index = exprToIndexInGroupByData.get(expr);
         if (index != null) {
-            if (currentGroupByExprData[index] != null) {
-                throw DbException.throwInternalError();
-            }
+            assert currentGroupByExprData[index] != null;
             currentGroupByExprData[index] = obj;
             return;
         }
@@ -368,73 +366,77 @@ public class Select extends Query {
         currentGroupByExprData = null;
         currentGroupsKey = null;
         exprToIndexInGroupByData.clear();
-        int rowNumber = 0;
-        setCurrentRowNumber(0);
-        ValueArray defaultGroup = ValueArray.get(new Value[0]);
-        int sampleSize = getSampleSizeValue(session);
-        while (topTableFilter.next()) {
-            setCurrentRowNumber(rowNumber + 1);
-            if (isConditionMet()) {
-                rowNumber++;
-                if (groupIndex == null) {
-                    currentGroupsKey = defaultGroup;
-                } else {
-                    Value[] keyValues = new Value[groupIndex.length];
-                    // update group
-                    for (int i = 0; i < groupIndex.length; i++) {
-                        int idx = groupIndex[i];
-                        Expression expr = expressions.get(idx);
-                        keyValues[i] = expr.getValue(session);
+        try {
+            int rowNumber = 0;
+            setCurrentRowNumber(0);
+            ValueArray defaultGroup = ValueArray.get(new Value[0]);
+            int sampleSize = getSampleSizeValue(session);
+            while (topTableFilter.next()) {
+                setCurrentRowNumber(rowNumber + 1);
+                if (isConditionMet()) {
+                    rowNumber++;
+                    if (groupIndex == null) {
+                        currentGroupsKey = defaultGroup;
+                    } else {
+                        Value[] keyValues = new Value[groupIndex.length];
+                        // update group
+                        for (int i = 0; i < groupIndex.length; i++) {
+                            int idx = groupIndex[i];
+                            Expression expr = expressions.get(idx);
+                            keyValues[i] = expr.getValue(session);
+                        }
+                        currentGroupsKey = ValueArray.get(keyValues);
+                    }
+                    Object[] values = groupByData.get(currentGroupsKey);
+                    if (values == null) {
+                        values = new Object[Math.max(exprToIndexInGroupByData.size(), expressions.size())];
+                        groupByData.put(currentGroupsKey, values);
+                    }
+                    currentGroupByExprData = values;
+                    currentGroupRowId++;
+                    for (int i = 0; i < columnCount; i++) {
+                        if (groupByExpression == null || !groupByExpression[i]) {
+                            Expression expr = expressions.get(i);
+                            expr.updateAggregate(session);
+                        }
+                    }
+                    if (sampleSize > 0 && rowNumber >= sampleSize) {
+                        break;
                     }
-                    currentGroupsKey = ValueArray.get(keyValues);
                 }
-                Object[] values = groupByData.get(currentGroupsKey);
-                if (values == null) {
-                    values = new Object[Math.max(exprToIndexInGroupByData.size(), expressions.size())];
-                    groupByData.put(currentGroupsKey, values);
+            }
+            if (groupIndex == null && groupByData.size() == 0) {
+                groupByData.put(defaultGroup,
+                        new Object[Math.max(exprToIndexInGroupByData.size(), expressions.size())]);
+            }
+            ArrayList<Value> keys = groupByData.keys();
+            for (Value v : keys) {
+                currentGroupsKey = (ValueArray) v;
+                currentGroupByExprData = groupByData.get(currentGroupsKey);
+                Value[] keyValues = currentGroupsKey.getList();
+                Value[] row = new Value[columnCount];
+                for (int j = 0; groupIndex != null && j < groupIndex.length; j++) {
+                    row[groupIndex[j]] = keyValues[j];
                 }
-                currentGroupByExprData = values;
-                currentGroupRowId++;
-                for (int i = 0; i < columnCount; i++) {
-                    if (groupByExpression == null || !groupByExpression[i]) {
-                        Expression expr = expressions.get(i);
-                        expr.updateAggregate(session);
+                for (int j = 0; j < columnCount; j++) {
+                    if (groupByExpression != null && groupByExpression[j]) {
+                        continue;
                     }
+                    Expression expr = expressions.get(j);
+                    row[j] = expr.getValue(session);
                 }
-                if (sampleSize > 0 && rowNumber >= sampleSize) {
-                    break;
-                }
-            }
-        }
-        if (groupIndex == null && groupByData.size() == 0) {
-            groupByData.put(defaultGroup, new Object[Math.max(exprToIndexInGroupByData.size(), expressions.size())]);
-        }
-        ArrayList<Value> keys = groupByData.keys();
-        for (Value v : keys) {
-            currentGroupsKey = (ValueArray) v;
-            currentGroupByExprData = groupByData.get(currentGroupsKey);
-            Value[] keyValues = currentGroupsKey.getList();
-            Value[] row = new Value[columnCount];
-            for (int j = 0; groupIndex != null && j < groupIndex.length; j++) {
-                row[groupIndex[j]] = keyValues[j];
-            }
-            for (int j = 0; j < columnCount; j++) {
-                if (groupByExpression != null && groupByExpression[j]) {
+                if (isHavingNullOrFalse(row)) {
                     continue;
                 }
-                Expression expr = expressions.get(j);
-                row[j] = expr.getValue(session);
+                row = keepOnlyDistinct(row, columnCount);
+                result.addRow(row);
             }
-            if (isHavingNullOrFalse(row)) {
-                continue;
-            }
-            row = keepOnlyDistinct(row, columnCount);
-            result.addRow(row);
+        } finally {
+            groupByData = null;
+            currentGroupsKey = null;
+            currentGroupByExprData = null;
+            exprToIndexInGroupByData.clear();
         }
-        groupByData = null;
-        currentGroupsKey = null;
-        currentGroupByExprData = null;
-        exprToIndexInGroupByData.clear();
     }
 
     /**

h2/src/main/org/h2/util/ValueHashMap.java

@@ -19,14 +19,13 @@ import org.h2.value.ValueNull;
  * <p>
  * HashMap in archaic versions of Java have some overhead for allocation of
  * entries, but slightly better behaviour with limited number of collisions,
- * because collisions have no impact on non-colliding entires. HashMap in modern
+ * because collisions have no impact on non-colliding entries. HashMap in modern
  * versions of Java also have the same overhead, but it builds a trees of keys
  * with colliding hashes, that's why even if the all keys have exactly the same
  * hash code it still offers a good performance similar to TreeMap. So
  * ValueHashMap is faster in typical cases, but may behave really bad in some
  * cases. HashMap is slower in typical cases, but its performance does not
- * degrade too much even in the worst possible case (if keys are comparable,
- * like our Value class).
+ * degrade too much even in the worst possible case (if keys are comparable).
  *
  * @param <V> the value type
  */