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-843-rc2 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));
