Merge pull request #1462 from katzyn/window

Separate aggregate and window code in some places

h2/src/docsrc/html/grammar.html

@@ -256,7 +256,12 @@ ${item.example}</p>
 <c:forEach var="item" items="otherGrammar">
 <h3 id="${item.link}" class="notranslate" onclick="switchBnf(this)">${item.topic}</h3>
 <!-- railroad-start -->
+<pre name="bnf" style="display: none">
+${item.syntax}
+</pre>
+<div name="railroad">
 ${item.railroad}
+</div>
 <!-- railroad-end -->
 <!-- syntax-start
 <pre>

h2/src/main/org/h2/command/Parser.java

@@ -171,6 +171,7 @@ import org.h2.expression.UnaryOperation;
 import org.h2.expression.ValueExpression;
 import org.h2.expression.Variable;
 import org.h2.expression.Wildcard;
+import org.h2.expression.aggregate.DataAnalysisOperation;
 import org.h2.expression.aggregate.AbstractAggregate;
 import org.h2.expression.aggregate.Aggregate;
 import org.h2.expression.aggregate.Aggregate.AggregateType;
@@ -3053,23 +3054,24 @@ public class Parser {
     }
 
     private void readFilterAndOver(AbstractAggregate aggregate) {
-        boolean isAggregate = aggregate.isAggregate();
-        if (isAggregate && readIf("FILTER")) {
+        if (readIf("FILTER")) {
             read(OPEN_PAREN);
             read(WHERE);
             Expression filterCondition = readExpression();
             read(CLOSE_PAREN);
             aggregate.setFilterCondition(filterCondition);
         }
-        Window over = null;
+        readOver(aggregate);
+    }
+
+    private void readOver(DataAnalysisOperation operation) {
         if (readIf("OVER")) {
-            over = readWindowNameOrSpecification();
-            aggregate.setOverCondition(over);
+            operation.setOverCondition(readWindowNameOrSpecification());
             currentSelect.setWindowQuery();
-        } else if (!isAggregate) {
-            throw getSyntaxError();
-        } else {
+        } else if (operation.isAggregate()) {
             currentSelect.setGroupQuery();
+        } else {
+            throw getSyntaxError();
         }
     }
 
@@ -3440,7 +3442,7 @@ public class Parser {
         default:
             // Avoid warning
         }
-        readFilterAndOver(function);
+        readOver(function);
         return function;
     }
 

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

@@ -14,6 +14,7 @@ import java.util.Map.Entry;
 
 import org.h2.engine.Session;
 import org.h2.expression.Expression;
+import org.h2.expression.aggregate.DataAnalysisOperation;
 import org.h2.value.Value;
 import org.h2.value.ValueArray;
 
@@ -210,7 +211,7 @@ public abstract class SelectGroups {
     /**
      * Maps an expression object to its data.
      */
-    private final HashMap<Expression, Object> windowData = new HashMap<>();
+    private final HashMap<DataAnalysisOperation, Object> windowData = new HashMap<>();
 
     /**
      * The id of the current group.
@@ -251,14 +252,9 @@ public abstract class SelectGroups {
      *
      * @param expr
      *            expression
-     * @param window
-     *            true if expression is a window expression
      * @return expression data or null
      */
-    public Object getCurrentGroupExprData(Expression expr, boolean window) {
-        if (window) {
-            return windowData.get(expr);
-        }
+    public final Object getCurrentGroupExprData(Expression expr) {
         Integer index = exprToIndexInGroupByData.get(expr);
         if (index == null) {
             return null;
@@ -273,15 +269,8 @@ public abstract class SelectGroups {
      *            expression
      * @param object
      *            expression data to set
-     * @param window
-     *            true if expression is a window expression
      */
-    public void setCurrentGroupExprData(Expression expr, Object obj, boolean window) {
-        if (window) {
-            Object old = windowData.put(expr, obj);
-            assert old == null;
-            return;
-        }
+    public final void setCurrentGroupExprData(Expression expr, Object obj) {
         Integer index = exprToIndexInGroupByData.get(expr);
         if (index != null) {
             assert currentGroupByExprData[index] == null;
@@ -297,6 +286,30 @@ public abstract class SelectGroups {
         currentGroupByExprData[index] = obj;
     }
 
+    /**
+     * Get the window data for the specified expression.
+     *
+     * @param expr
+     *            expression
+     * @return expression data or null
+     */
+    public final Object getWindowExprData(DataAnalysisOperation expr) {
+        return windowData.get(expr);
+    }
+
+    /**
+     * Set the window data for the specified expression.
+     *
+     * @param expr
+     *            expression
+     * @param object
+     *            expression data to set
+     */
+    public final void setWindowExprData(DataAnalysisOperation expr, Object obj) {
+        Object old = windowData.put(expr, obj);
+        assert old == null;
+    }
+
     abstract void updateCurrentGroupExprData();
 
     /**

h2/src/main/org/h2/expression/ExpressionColumn.java

@@ -164,9 +164,9 @@ public class ExpressionColumn extends Expression {
             // this is a different level (the enclosing query)
             return;
         }
-        Value v = (Value) groupData.getCurrentGroupExprData(this, false);
+        Value v = (Value) groupData.getCurrentGroupExprData(this);
         if (v == null) {
-            groupData.setCurrentGroupExprData(this, now, false);
+            groupData.setCurrentGroupExprData(this, now);
         } else {
             if (!database.areEqual(now, v)) {
                 throw DbException.get(ErrorCode.MUST_GROUP_BY_COLUMN_1, getSQL());
@@ -180,7 +180,7 @@ public class ExpressionColumn extends Expression {
         if (select != null) {
             SelectGroups groupData = select.getGroupDataIfCurrent(false);
             if (groupData != null) {
-                Value v = (Value) groupData.getCurrentGroupExprData(this, false);
+                Value v = (Value) groupData.getCurrentGroupExprData(this);
                 if (v != null) {
                     return v;
                 }

h2/src/main/org/h2/expression/aggregate/Aggregate.java

@@ -251,11 +251,6 @@ public class Aggregate extends AbstractAggregate {
         return AGGREGATES.get(name);
     }
 
-    @Override
-    public boolean isAggregate() {
-        return true;
-    }
-
     /**
      * Set the order for ARRAY_AGG() or GROUP_CONCAT() aggregate.
      *
@@ -312,6 +307,7 @@ public class Aggregate extends AbstractAggregate {
 
     @Override
     protected void updateGroupAggregates(Session session, int stage) {
+        super.updateGroupAggregates(session, stage);
         if (on != null) {
             on.updateAggregate(session, stage);
         }
@@ -514,9 +510,6 @@ public class Aggregate extends AbstractAggregate {
         if (groupConcatSeparator != null) {
             groupConcatSeparator = groupConcatSeparator.optimize(session);
         }
-        if (filterCondition != null) {
-            filterCondition = filterCondition.optimize(session);
-        }
         switch (type) {
         case GROUP_CONCAT:
             dataType = Value.STRING;

h2/src/main/org/h2/expression/aggregate/JavaAggregate.java

@@ -40,11 +40,6 @@ public class JavaAggregate extends AbstractAggregate {
         this.args = args;
     }
 
-    @Override
-    public boolean isAggregate() {
-        return true;
-    }
-
     @Override
     public int getCost() {
         int cost = 5;
@@ -140,9 +135,6 @@ public class JavaAggregate extends AbstractAggregate {
         } catch (SQLException e) {
             throw DbException.convert(e);
         }
-        if (filterCondition != null) {
-            filterCondition = filterCondition.optimize(session);
-        }
         return this;
     }
 
@@ -237,6 +229,7 @@ public class JavaAggregate extends AbstractAggregate {
 
     @Override
     protected void updateGroupAggregates(Session session, int stage) {
+        super.updateGroupAggregates(session, stage);
         for (Expression expr : args) {
             expr.updateAggregate(session, stage);
         }

h2/src/main/org/h2/expression/aggregate/WindowFunction.java

@@ -10,6 +10,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 
 import org.h2.command.dml.Select;
+import org.h2.command.dml.SelectGroups;
 import org.h2.command.dml.SelectOrderBy;
 import org.h2.engine.Session;
 import org.h2.expression.Expression;
@@ -24,7 +25,7 @@ import org.h2.value.ValueNull;
 /**
  * A window function.
  */
-public class WindowFunction extends AbstractAggregate {
+public class WindowFunction extends DataAnalysisOperation {
 
     private final WindowFunctionType type;
 
@@ -105,7 +106,7 @@ public class WindowFunction extends AbstractAggregate {
      *            arguments, or null
      */
     public WindowFunction(WindowFunctionType type, Select select, Expression[] args) {
-        super(select, false);
+        super(select);
         this.type = type;
         this.args = args;
     }
@@ -145,12 +146,13 @@ public class WindowFunction extends AbstractAggregate {
     }
 
     @Override
-    protected void updateAggregate(Session session, Object aggregateData) {
-        throw DbException.getUnsupportedException("Window function");
+    protected void updateAggregate(Session session, SelectGroups groupData, int groupRowId) {
+        updateOrderedAggregate(session, groupData, groupRowId, over.getOrderBy());
     }
 
     @Override
     protected void updateGroupAggregates(Session session, int stage) {
+        super.updateGroupAggregates(session, stage);
         if (args != null) {
             for (Expression expr : args) {
                 expr.updateAggregate(session, stage);

h2/src/main/org/h2/mvstore/tx/TransactionStore.java

@@ -449,9 +449,10 @@ public class TransactionStore {
                         if (map != null) { // might be null if map was removed later
                             Object key = op[1];
                             commitDecisionMaker.setUndoKey(undoKey);
-                            // although second parameter (value) is not really used
-                            // by CommitDecisionMaker, MVRTreeMap has weird traversal logic based on it,
-                            // and any non-null value will do, to signify update, not removal
+                            // although second parameter (value) is not really
+                            // used by CommitDecisionMaker, MVRTreeMap has weird
+                            // traversal logic based on it, and any non-null
+                            // value will do, to signify update, not removal
                             map.operate(key, VersionedValue.DUMMY, commitDecisionMaker);
                         }
                     }

h2/src/tools/org/h2/build/doc/dictionary.txt

@@ -796,4 +796,4 @@ interior envelopes multilinestring multipoint packed exterior normalization awkw
 xym normalizes coord setz xyzm geometrycollection multipolygon mixup rings polygons rejection finite
 pointzm pointz pointm dimensionality redefine forum measures
 mpg casted pzm mls constrained subtypes complains
-ranks rno dro rko precede cume reopens preceding unbounded rightly itr lag maximal tiles tile ntile
+ranks rno dro rko precede cume reopens preceding unbounded rightly itr lag maximal tiles tile ntile signify