Repository: ignite
Updated Branches:
  refs/heads/ignite-1.5.1-2 b534c4271 -> c0cfc6a78


ignite-2205 - clause HAVING doesn't work for count - Fixes #357.

Signed-off-by: S.Vladykin <svlady...@gridgain.com>


Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/a0cdb596
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/a0cdb596
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/a0cdb596

Branch: refs/heads/ignite-1.5.1-2
Commit: a0cdb59645ce0a84344c6c92b9695c1a26e2a824
Parents: 81458f5
Author: S.Vladykin <svlady...@gridgain.com>
Authored: Mon Dec 21 19:58:29 2015 +0300
Committer: S.Vladykin <svlady...@gridgain.com>
Committed: Mon Dec 21 19:58:29 2015 +0300

----------------------------------------------------------------------
 .../processors/query/h2/sql/GridSqlQuery.java   |  3 +
 .../query/h2/sql/GridSqlQuerySplitter.java      | 65 ++++++++++++++------
 .../query/IgniteSqlSplitterSelfTest.java        | 52 ++++++++++++++++
 .../query/h2/sql/GridQueryParsingTest.java      |  9 +--
 4 files changed, 103 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ignite/blob/a0cdb596/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuery.java
----------------------------------------------------------------------
diff --git 
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuery.java
 
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuery.java
index ddcb40b..d9784c8 100644
--- 
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuery.java
+++ 
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuery.java
@@ -163,6 +163,9 @@ public abstract class GridSqlQuery {
                     if (expr == null) // For plain select should never be 
null, for union H2 itself can't parse query.
                         throw new IllegalStateException("Failed to build 
query: " + buff.toString());
 
+                    if (expr instanceof GridSqlAlias)
+                        expr = expr.child();
+
                     
buff.append('=').append(StringUtils.unEnclose(expr.getSQL()));
                 }
 

http://git-wip-us.apache.org/repos/asf/ignite/blob/a0cdb596/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
----------------------------------------------------------------------
diff --git 
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
 
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
index 0c9c8fe..727c2c7 100644
--- 
a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
+++ 
b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlQuerySplitter.java
@@ -50,6 +50,9 @@ public class GridSqlQuerySplitter {
     /** */
     private static final String COLUMN_PREFIX = "__C";
 
+    /** */
+    private static final String HAVING_COLUMN = "__H";
+
     /**
      * @param idx Index of table.
      * @return Table.
@@ -158,14 +161,17 @@ public class GridSqlQuerySplitter {
         List<GridSqlElement> mapExps = F.addAll(new 
ArrayList<GridSqlElement>(mapQry.allColumns()),
             mapQry.columns(false));
 
-        GridSqlElement[] rdcExps = new GridSqlElement[mapQry.visibleColumns()];
+        final int visibleCols = mapQry.visibleColumns();
+        final int havingCol = mapQry.havingColumn();
+
+        List<GridSqlElement> rdcExps = new ArrayList<>(visibleCols);
 
         Set<String> colNames = new HashSet<>();
 
         boolean aggregateFound = false;
 
         for (int i = 0, len = mapExps.size(); i < len; i++) // Remember len 
because mapExps list can grow.
-            aggregateFound |= splitSelectExpression(mapExps, rdcExps, 
colNames, i, collocated);
+            aggregateFound |= splitSelectExpression(mapExps, rdcExps, 
colNames, i, collocated, i == havingCol);
 
         // Fill select expressions.
         mapQry.clearColumns();
@@ -173,10 +179,13 @@ public class GridSqlQuerySplitter {
         for (GridSqlElement exp : mapExps) // Add all map expressions as 
visible.
             mapQry.addColumn(exp, true);
 
-        for (GridSqlElement rdcExp : rdcExps) // Add corresponding visible 
reduce columns.
-            rdcQry.addColumn(rdcExp, true);
+        for (int i = 0; i < visibleCols; i++) // Add visible reduce columns.
+            rdcQry.addColumn(rdcExps.get(i), true);
+
+        for (int i = visibleCols; i < rdcExps.size(); i++) // Add invisible 
reduce columns (HAVING).
+            rdcQry.addColumn(rdcExps.get(i), false);
 
-        for (int i = rdcExps.length; i < mapExps.size(); i++)  // Add all 
extra map columns as invisible reduce columns.
+        for (int i = rdcExps.size(); i < mapExps.size(); i++)  // Add all 
extra map columns as invisible reduce columns.
             rdcQry.addColumn(column(((GridSqlAlias)mapExps.get(i)).alias()), 
false);
 
         // -- GROUP BY
@@ -184,9 +193,18 @@ public class GridSqlQuerySplitter {
             rdcQry.groupColumns(mapQry.groupColumns());
 
         // -- HAVING
-        if (mapQry.havingColumn() >= 0 && !collocated) {
+        if (havingCol >= 0 && !collocated) {
             // TODO IGNITE-1140 - Find aggregate functions in HAVING clause or 
rewrite query to put all aggregates to SELECT clause.
-            rdcQry.whereAnd(column(columnName(mapQry.havingColumn())));
+            // We need to find HAVING column in reduce query.
+            for (int i = visibleCols; i < rdcQry.allColumns(); i++) {
+                GridSqlElement c = rdcQry.column(i);
+
+                if (c instanceof GridSqlAlias && 
HAVING_COLUMN.equals(((GridSqlAlias)c).alias())) {
+                    rdcQry.havingColumn(i);
+
+                    break;
+                }
+            }
 
             mapQry.havingColumn(-1);
         }
@@ -452,10 +470,11 @@ public class GridSqlQuerySplitter {
      * @param colNames Set of unique top level column names.
      * @param idx Index.
      * @param collocated If it is a collocated query.
+     * @param isHaving If it is a HAVING expression.
      * @return {@code true} If aggregate was found.
      */
-    private static boolean splitSelectExpression(List<GridSqlElement> 
mapSelect, GridSqlElement[] rdcSelect,
-        Set<String> colNames, final int idx, boolean collocated) {
+    private static boolean splitSelectExpression(List<GridSqlElement> 
mapSelect, List<GridSqlElement> rdcSelect,
+        Set<String> colNames, final int idx, boolean collocated, boolean 
isHaving) {
         GridSqlElement el = mapSelect.get(idx);
 
         GridSqlAlias alias = null;
@@ -471,16 +490,15 @@ public class GridSqlQuerySplitter {
             aggregateFound = true;
 
             if (alias == null)
-                alias = alias(columnName(idx), el);
+                alias = alias(isHaving ? HAVING_COLUMN : columnName(idx), el);
 
             // We can update original alias here as well since it will be 
dropped from mapSelect.
             splitAggregates(alias, 0, mapSelect, idx, true);
 
-            if (idx < rdcSelect.length)
-                rdcSelect[idx] = alias;
+            set(rdcSelect, idx, alias);
         }
         else {
-            String mapColAlias = columnName(idx);
+            String mapColAlias = isHaving ? HAVING_COLUMN : columnName(idx);
             String rdcColAlias;
 
             if (alias == null)  // Original column name for reduce column.
@@ -491,20 +509,29 @@ public class GridSqlQuerySplitter {
             // Always wrap map column into generated alias.
             mapSelect.set(idx, alias(mapColAlias, el)); // `el` is known not 
to be an alias.
 
-            if (idx < rdcSelect.length) { // SELECT __C0 AS original_alias
-                GridSqlElement rdcEl = column(mapColAlias);
+            // SELECT __C0 AS original_alias
+            GridSqlElement rdcEl = column(mapColAlias);
 
-                if (colNames.add(rdcColAlias)) // To handle column name 
duplication (usually wildcard for few tables).
-                    rdcEl = alias(rdcColAlias, rdcEl);
+            if (colNames.add(rdcColAlias)) // To handle column name 
duplication (usually wildcard for few tables).
+                rdcEl = alias(rdcColAlias, rdcEl);
 
-                rdcSelect[idx] = rdcEl;
-            }
+            set(rdcSelect, idx, rdcEl);
         }
 
         return aggregateFound;
     }
 
     /**
+     * @param list List.
+     * @param idx Index.
+     * @param item Element.
+     */
+    private static <Z> void set(List<Z> list, int idx, Z item) {
+        assert list.size() == idx;
+        list.add(item);
+    }
+
+    /**
      * @param el Expression.
      * @return {@code true} If expression contains aggregates.
      */

http://git-wip-us.apache.org/repos/asf/ignite/blob/a0cdb596/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
----------------------------------------------------------------------
diff --git 
a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
 
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
index 5702649..d0e2780 100644
--- 
a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
+++ 
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/IgniteSqlSplitterSelfTest.java
@@ -20,8 +20,11 @@ package org.apache.ignite.internal.processors.query;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Random;
+import java.util.concurrent.atomic.AtomicLong;
 import org.apache.ignite.IgniteCache;
 import org.apache.ignite.cache.CacheAtomicityMode;
 import org.apache.ignite.cache.CacheMode;
@@ -31,6 +34,7 @@ import org.apache.ignite.configuration.CacheConfiguration;
 import org.apache.ignite.configuration.IgniteConfiguration;
 import org.apache.ignite.internal.util.GridRandom;
 import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.X;
 import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
 import org.apache.ignite.spi.discovery.tcp.ipfinder.TcpDiscoveryIpFinder;
 import org.apache.ignite.spi.discovery.tcp.ipfinder.vm.TcpDiscoveryVmIpFinder;
@@ -203,6 +207,54 @@ public class IgniteSqlSplitterSelfTest extends 
GridCommonAbstractTest {
     }
 
     /**
+     * Test HAVING clause.
+     */
+    public void testHaving() {
+        IgniteCache<Integer, Integer> c = 
ignite(0).getOrCreateCache(cacheConfig("ints", true,
+            Integer.class, Integer.class));
+
+        try {
+            Random rnd = new GridRandom();
+
+            Map<Integer, AtomicLong> cntMap = new HashMap<>();
+
+            for (int i = 0; i < 1000; i++) {
+                int v = (int)(50 * rnd.nextGaussian());
+
+                c.put(i, v);
+
+                AtomicLong cnt = cntMap.get(v);
+
+                if (cnt == null)
+                    cntMap.put(v, cnt = new AtomicLong());
+
+                cnt.incrementAndGet();
+            }
+
+            assertTrue(cntMap.size() > 10);
+
+            String sqlQry = "select _val, count(*) cnt from Integer group by 
_val having cnt > ?";
+
+            X.println("Plan: " + c.query(new SqlFieldsQuery("explain " + 
sqlQry).setArgs(0)).getAll());
+
+            for (int i = -1; i <= 1001; i += 10) {
+                List<List<?>> res = c.query(new 
SqlFieldsQuery(sqlQry).setArgs(i)).getAll();
+
+                for (List<?> row : res) {
+                    int v = (Integer)row.get(0);
+                    long cnt = (Long)row.get(1);
+
+                    assertTrue(cnt + " > " + i, cnt > i);
+                    assertEquals(cntMap.get(v).longValue(), cnt);
+                }
+            }
+        }
+        finally {
+            c.destroy();
+        }
+    }
+
+    /**
      * @param c Cache.
      * @param qry Query.
      * @param args Arguments.

http://git-wip-us.apache.org/repos/asf/ignite/blob/a0cdb596/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java
----------------------------------------------------------------------
diff --git 
a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java
 
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java
index 2be5d1a..ce1f241 100644
--- 
a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java
+++ 
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java
@@ -199,7 +199,7 @@ public class GridQueryParsingTest extends 
GridCommonAbstractTest {
 
         checkQuery("select p.name, ? from Person p where name regexp ? and 
p.old < ?");
 
-        checkQuery("select count(*) as a from Person");
+        checkQuery("select count(*) as a from Person having a > 10");
         checkQuery("select count(*) as a, count(p.*), count(p.name) from 
Person p");
         checkQuery("select count(distinct p.name) from Person p");
         checkQuery("select name, count(*) cnt from Person group by name order 
by cnt desc limit 10");
@@ -316,12 +316,7 @@ public class GridQueryParsingTest extends 
GridCommonAbstractTest {
 
         GridSqlQueryParser ses = new GridSqlQueryParser();
 
-        String res;
-
-        if (prepared instanceof Query)
-            res = ses.parse((Query) prepared).getSQL();
-        else
-            throw new UnsupportedOperationException();
+        String res = ses.parse(prepared).getSQL();
 
         System.out.println(normalizeSql(res));
 

Reply via email to