Skip to content

Commit

Permalink
Fix multi-shard SELECT problem with HAVING clauses
Browse files Browse the repository at this point in the history
Because Vars from HAVING clauses weren't being included in the remote
query's target list, they would be NULL if they did not appear in the
target list or a GROUP/ORDER clause. This change includes them in the
remote target list so they'll be available for local evaluation of the
HAVING clause.
  • Loading branch information
jasonmp85 committed Jan 22, 2015
1 parent 9fa64ed commit ca9308c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
15 changes: 15 additions & 0 deletions expected/queries.out
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ SELECT COUNT(*) FROM articles;
50
(1 row)

-- try query with more SQL features
SELECT author_id, sum(word_count) AS corpus_size FROM articles
GROUP BY author_id
HAVING sum(word_count) > 25000
Expand Down Expand Up @@ -220,6 +221,20 @@ LOG: distributed statement: SELECT NULL::unknown FROM ONLY articles_10036 WHERE

SET client_min_messages = DEFAULT;
SET pg_shard.log_distributed_statements = DEFAULT;
-- use HAVING without its variable in target list
SELECT author_id FROM articles
GROUP BY author_id
HAVING sum(word_count) > 50000
ORDER BY author_id;
author_id
-----------
2
4
6
8
10
(5 rows)

-- verify temp tables used by cross-shard queries do not persist
SELECT COUNT(*) FROM pg_class WHERE relname LIKE 'pg_shard_temp_table%' AND
relkind = 'r';
Expand Down
8 changes: 7 additions & 1 deletion pg_shard.c
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ RowAndColumnFilterQuery(Query *query, List *remoteRestrictList, List *localRestr
List *rangeTableList = NIL;
List *whereColumnList = NIL;
List *projectColumnList = NIL;
List *havingClauseColumnList = NIL;
List *requiredColumnList = NIL;
ListCell *columnCell = NULL;
List *uniqueColumnList = NIL;
Expand All @@ -731,11 +732,16 @@ RowAndColumnFilterQuery(Query *query, List *remoteRestrictList, List *localRestr

/* as well as any used in projections (GROUP BY, etc.) */
projectColumnList = pull_var_clause((Node *) query->targetList, aggregateBehavior,
placeHolderBehavior);
placeHolderBehavior);

/* finally, need those used in any HAVING quals */
havingClauseColumnList = pull_var_clause(query->havingQual, aggregateBehavior,
placeHolderBehavior);

/* put them together to get list of required columns for query */
requiredColumnList = list_concat(requiredColumnList, whereColumnList);
requiredColumnList = list_concat(requiredColumnList, projectColumnList);
requiredColumnList = list_concat(requiredColumnList, havingClauseColumnList);

/* ensure there are no duplicates in the list */
foreach(columnCell, requiredColumnList)
Expand Down
7 changes: 7 additions & 0 deletions sql/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ SELECT * FROM (articles INNER JOIN authors ON articles.id = authors.id);
-- test cross-shard queries
SELECT COUNT(*) FROM articles;

-- try query with more SQL features
SELECT author_id, sum(word_count) AS corpus_size FROM articles
GROUP BY author_id
HAVING sum(word_count) > 25000
Expand All @@ -146,6 +147,12 @@ SELECT count(*) FROM articles WHERE word_count > 10000;
SET client_min_messages = DEFAULT;
SET pg_shard.log_distributed_statements = DEFAULT;

-- use HAVING without its variable in target list
SELECT author_id FROM articles
GROUP BY author_id
HAVING sum(word_count) > 50000
ORDER BY author_id;

-- verify temp tables used by cross-shard queries do not persist
SELECT COUNT(*) FROM pg_class WHERE relname LIKE 'pg_shard_temp_table%' AND
relkind = 'r';
Expand Down

0 comments on commit ca9308c

Please sign in to comment.