Merge pull request #2258 from citusdata/release-6.2-respect_enable_hashagg

Release 6.2 respect enable hashagg

.travis.yml

@@ -14,7 +14,7 @@ env:
     - PGVERSION=9.5
     - PGVERSION=9.6
 before_install:
-  - git clone -b v0.6.1 --depth 1 https://github.com/citusdata/tools.git
+  - git clone -b v0.7.6 --depth 1 https://github.com/citusdata/tools.git
   - sudo make -C tools install
   - setup_apt
   - curl https://install.citusdata.com/community/deb.sh | sudo bash

src/backend/distributed/planner/multi_explain.c

@@ -492,26 +492,26 @@ BuildRemoteExplainQuery(char *queryString, ExplainState *es)
 		case EXPLAIN_FORMAT_XML:
 		{
 			formatStr = "XML";
+			break;
 		}
-		break;
 
 		case EXPLAIN_FORMAT_JSON:
 		{
 			formatStr = "JSON";
+			break;
 		}
-		break;
 
 		case EXPLAIN_FORMAT_YAML:
 		{
 			formatStr = "YAML";
+			break;
 		}
-		break;
 
 		default:
 		{
 			formatStr = "TEXT";
+			break;
 		}
-		break;
 	}
 
 	appendStringInfo(explainQuery,

src/backend/distributed/planner/multi_master_planner.c

@@ -21,6 +21,7 @@
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
+#include "optimizer/cost.h"
 #include "optimizer/planmain.h"
 #include "optimizer/tlist.h"
 #include "optimizer/var.h"
@@ -128,13 +129,47 @@ BuildAggregatePlan(Query *masterQuery, Plan *subPlan)
 	/* if we have grouping, then initialize appropriate information */
 	if (groupColumnCount > 0)
 	{
-		if (!grouping_is_hashable(groupColumnList))
+		bool groupingIsHashable = grouping_is_hashable(groupColumnList);
+		bool groupingIsSortable = grouping_is_sortable(groupColumnList);
+
+		if (!groupingIsHashable && !groupingIsSortable)
 		{
-			ereport(ERROR, (errmsg("grouped column list cannot be hashed")));
+			ereport(ERROR, (errmsg("grouped column list cannot be hashed or sorted")));
 		}
 
-		/* switch to hashed aggregate strategy to allow grouping */
+		/*
-		aggregateStrategy = AGG_HASHED;
+		 * Postgres hash aggregate strategy does not support distinct aggregates
+		 * in group and order by with aggregate operations.
+		 * see nodeAgg.c:build_pertrans_for_aggref(). In that case we use
+		 * sorted agg strategy, otherwise we use hash strategy.
+		 */
+		if (!enable_hashagg || !groupingIsHashable)
+		{
+			char *messageHint = NULL;
+			if (!enable_hashagg && groupingIsHashable)
+			{
+				messageHint = "Consider setting enable_hashagg to on.";
+			}
+
+			if (!groupingIsSortable)
+			{
+				ereport(ERROR, (errmsg("grouped column list must cannot be sorted"),
+								errdetail("Having a distinct aggregate requires "
+										  "grouped column list to be sortable."),
+								messageHint ? errhint("%s", messageHint) : 0));
+			}
+
+			aggregateStrategy = AGG_SORTED;
+#if (PG_VERSION_NUM >= 90600)
+			subPlan = (Plan *) make_sort_from_sortclauses(groupColumnList, subPlan);
+#else
+			subPlan = (Plan *) make_sort_from_sortclauses(NULL, groupColumnList, subPlan);
+#endif
+		}
+		else
+		{
+			aggregateStrategy = AGG_HASHED;
+		}
 
 		/* get column indexes that are being grouped */
 		groupColumnIdArray = extract_grouping_cols(groupColumnList, subPlan->targetlist);

src/backend/distributed/planner/shard_pruning.c

@@ -632,8 +632,8 @@ AddPartitionKeyRestrictionToInstance(ClauseWalkerContext *context, OpExpr *opCla
 					prune->lessConsts = constantClause;
 				}
 				matchedOp = true;
+				break;
 			}
-			break;
 
 			case BTLessEqualStrategyNumber:
 			{
@@ -645,8 +645,8 @@ AddPartitionKeyRestrictionToInstance(ClauseWalkerContext *context, OpExpr *opCla
 					prune->lessEqualConsts = constantClause;
 				}
 				matchedOp = true;
+				break;
 			}
-			break;
 
 			case BTEqualStrategyNumber:
 			{
@@ -662,8 +662,8 @@ AddPartitionKeyRestrictionToInstance(ClauseWalkerContext *context, OpExpr *opCla
 					prune->evaluatesToFalse = true;
 				}
 				matchedOp = true;
+				break;
 			}
-			break;
 
 			case BTGreaterEqualStrategyNumber:
 			{
@@ -676,8 +676,8 @@ AddPartitionKeyRestrictionToInstance(ClauseWalkerContext *context, OpExpr *opCla
 					prune->greaterEqualConsts = constantClause;
 				}
 				matchedOp = true;
+				break;
 			}
-			break;
 
 			case BTGreaterStrategyNumber:
 			{
@@ -689,15 +689,15 @@ AddPartitionKeyRestrictionToInstance(ClauseWalkerContext *context, OpExpr *opCla
 					prune->greaterConsts = constantClause;
 				}
 				matchedOp = true;
+				break;
 			}
-			break;
 
 			case ROWCOMPARE_NE:
 			{
 				/* TODO: could add support for this, if we feel like it */
 				matchedOp = false;
+				break;
 			}
-			break;
 
 			default:
 				Assert(false);

src/backend/distributed/transaction/transaction_management.c

@@ -168,8 +168,8 @@ CoordinatedTransactionCallback(XactEvent event, void *arg)
 			XactModificationLevel = XACT_MODIFICATION_NONE;
 			dlist_init(&InProgressTransactions);
 			CoordinatedTransactionUses2PC = false;
+			break;
 		}
-		break;
 
 		case XACT_EVENT_ABORT:
 		{
@@ -204,8 +204,8 @@ CoordinatedTransactionCallback(XactEvent event, void *arg)
 			dlist_init(&InProgressTransactions);
 			CoordinatedTransactionUses2PC = false;
 			subXactAbortAttempted = false;
+			break;
 		}
-		break;
 
 		case XACT_EVENT_PARALLEL_COMMIT:
 		case XACT_EVENT_PARALLEL_ABORT:
@@ -270,8 +270,8 @@ CoordinatedTransactionCallback(XactEvent event, void *arg)
 			 * committed. This handles failure at COMMIT/PREPARE time.
 			 */
 			PostCommitMarkFailedShardPlacements(CoordinatedTransactionUses2PC);
+			break;
 		}
-		break;
 
 		case XACT_EVENT_PARALLEL_PRE_COMMIT:
 		case XACT_EVENT_PRE_PREPARE:
@@ -282,8 +282,8 @@ CoordinatedTransactionCallback(XactEvent event, void *arg)
 								errmsg("cannot use 2PC in transactions involving "
 									   "multiple servers")));
 			}
+			break;
 		}
-		break;
 	}
 }
 

src/test/regress/expected/multi_explain.out

@@ -686,3 +686,23 @@ Custom Scan (Citus Router)
         ->  Seq Scan on explain_table_570001 explain_table
               Filter: (id = 1)
 ROLLBACK;
+-- Test disable hash aggregate
+SET enable_hashagg TO off;
+EXPLAIN (COSTS FALSE, FORMAT TEXT)
+	SELECT l_quantity, count(*) count_quantity FROM lineitem
+	GROUP BY l_quantity ORDER BY count_quantity, l_quantity;
+Sort
+  Sort Key: COALESCE((pg_catalog.sum((COALESCE((pg_catalog.sum(remote_scan.count_quantity))::bigint, '0'::bigint))))::bigint, '0'::bigint), remote_scan.l_quantity
+  ->  GroupAggregate
+        Group Key: remote_scan.l_quantity
+        ->  Sort
+              Sort Key: remote_scan.l_quantity
+              ->  Custom Scan (Citus Real-Time)
+                    Task Count: 8
+                    Tasks Shown: One of 8
+                    ->  Task
+                          Node: host=localhost port=57637 dbname=regression
+                          ->  HashAggregate
+                                Group Key: l_quantity
+                                ->  Seq Scan on lineitem_290001 lineitem
+SET enable_hashagg TO on;

src/test/regress/expected/multi_explain_0.out

@@ -657,3 +657,23 @@ Custom Scan (Citus Router)
         ->  Seq Scan on explain_table_570001 explain_table
               Filter: (id = 1)
 ROLLBACK;
+-- Test disable hash aggregate
+SET enable_hashagg TO off;
+EXPLAIN (COSTS FALSE, FORMAT TEXT)
+	SELECT l_quantity, count(*) count_quantity FROM lineitem
+	GROUP BY l_quantity ORDER BY count_quantity, l_quantity;
+Sort
+  Sort Key: COALESCE((sum((COALESCE((sum(remote_scan.count_quantity))::bigint, '0'::bigint))))::bigint, '0'::bigint), remote_scan.l_quantity
+  ->  GroupAggregate
+        Group Key: remote_scan.l_quantity
+        ->  Sort
+              Sort Key: remote_scan.l_quantity
+              ->  Custom Scan (Citus Real-Time)
+                    Task Count: 8
+                    Tasks Shown: One of 8
+                    ->  Task
+                          Node: host=localhost port=57637 dbname=regression
+                          ->  HashAggregate
+                                Group Key: l_quantity
+                                ->  Seq Scan on lineitem_290001 lineitem
+SET enable_hashagg TO on;

src/test/regress/expected/multi_index_statements.out

@@ -98,23 +98,23 @@ CREATE INDEX CONCURRENTLY ON local_table(id);
 DROP TABLE local_table;
 -- Verify that all indexes got created on the master node and one of the workers
 SELECT * FROM pg_indexes WHERE tablename = 'lineitem' or tablename like 'index_test_%' ORDER BY indexname;
- schemaname |    tablename     |             indexname              | tablespace |                                                      indexdef                                                       
+ schemaname |    tablename     |             indexname              | tablespace |                                                          indexdef                                                          
-------------+------------------+------------------------------------+------------+---------------------------------------------------------------------------------------------------------------------
+------------+------------------+------------------------------------+------------+----------------------------------------------------------------------------------------------------------------------------
- public     | index_test_hash  | index_test_hash_index_a            |            | CREATE UNIQUE INDEX index_test_hash_index_a ON index_test_hash USING btree (a)
+ public     | index_test_hash  | index_test_hash_index_a            |            | CREATE UNIQUE INDEX index_test_hash_index_a ON public.index_test_hash USING btree (a)
- public     | index_test_hash  | index_test_hash_index_a_b          |            | CREATE UNIQUE INDEX index_test_hash_index_a_b ON index_test_hash USING btree (a, b)
+ public     | index_test_hash  | index_test_hash_index_a_b          |            | CREATE UNIQUE INDEX index_test_hash_index_a_b ON public.index_test_hash USING btree (a, b)
- public     | index_test_hash  | index_test_hash_index_a_b_partial  |            | CREATE UNIQUE INDEX index_test_hash_index_a_b_partial ON index_test_hash USING btree (a, b) WHERE (c IS NOT NULL)
+ public     | index_test_hash  | index_test_hash_index_a_b_partial  |            | CREATE UNIQUE INDEX index_test_hash_index_a_b_partial ON public.index_test_hash USING btree (a, b) WHERE (c IS NOT NULL)
- public     | index_test_range | index_test_range_index_a           |            | CREATE UNIQUE INDEX index_test_range_index_a ON index_test_range USING btree (a)
+ public     | index_test_range | index_test_range_index_a           |            | CREATE UNIQUE INDEX index_test_range_index_a ON public.index_test_range USING btree (a)
- public     | index_test_range | index_test_range_index_a_b         |            | CREATE UNIQUE INDEX index_test_range_index_a_b ON index_test_range USING btree (a, b)
+ public     | index_test_range | index_test_range_index_a_b         |            | CREATE UNIQUE INDEX index_test_range_index_a_b ON public.index_test_range USING btree (a, b)
- public     | index_test_range | index_test_range_index_a_b_partial |            | CREATE UNIQUE INDEX index_test_range_index_a_b_partial ON index_test_range USING btree (a, b) WHERE (c IS NOT NULL)
+ public     | index_test_range | index_test_range_index_a_b_partial |            | CREATE UNIQUE INDEX index_test_range_index_a_b_partial ON public.index_test_range USING btree (a, b) WHERE (c IS NOT NULL)
- public     | lineitem         | lineitem_colref_index              |            | CREATE INDEX lineitem_colref_index ON lineitem USING btree (record_ne(lineitem.*, NULL::record))
+ public     | lineitem         | lineitem_colref_index              |            | CREATE INDEX lineitem_colref_index ON public.lineitem USING btree (record_ne(lineitem.*, NULL::record))
- public     | lineitem         | lineitem_concurrently_index        |            | CREATE INDEX lineitem_concurrently_index ON lineitem USING btree (l_orderkey)
+ public     | lineitem         | lineitem_concurrently_index        |            | CREATE INDEX lineitem_concurrently_index ON public.lineitem USING btree (l_orderkey)
- public     | lineitem         | lineitem_orderkey_hash_index       |            | CREATE INDEX lineitem_orderkey_hash_index ON lineitem USING hash (l_partkey)
+ public     | lineitem         | lineitem_orderkey_hash_index       |            | CREATE INDEX lineitem_orderkey_hash_index ON public.lineitem USING hash (l_partkey)
- public     | lineitem         | lineitem_orderkey_index            |            | CREATE INDEX lineitem_orderkey_index ON lineitem USING btree (l_orderkey)
+ public     | lineitem         | lineitem_orderkey_index            |            | CREATE INDEX lineitem_orderkey_index ON public.lineitem USING btree (l_orderkey)
- public     | lineitem         | lineitem_orderkey_index_new        |            | CREATE INDEX lineitem_orderkey_index_new ON lineitem USING btree (l_orderkey)
+ public     | lineitem         | lineitem_orderkey_index_new        |            | CREATE INDEX lineitem_orderkey_index_new ON public.lineitem USING btree (l_orderkey)
- public     | lineitem         | lineitem_partial_index             |            | CREATE INDEX lineitem_partial_index ON lineitem USING btree (l_shipdate) WHERE (l_shipdate < '01-01-1995'::date)
+ public     | lineitem         | lineitem_partial_index             |            | CREATE INDEX lineitem_partial_index ON public.lineitem USING btree (l_shipdate) WHERE (l_shipdate < '01-01-1995'::date)
- public     | lineitem         | lineitem_partkey_desc_index        |            | CREATE INDEX lineitem_partkey_desc_index ON lineitem USING btree (l_partkey DESC)
+ public     | lineitem         | lineitem_partkey_desc_index        |            | CREATE INDEX lineitem_partkey_desc_index ON public.lineitem USING btree (l_partkey DESC)
- public     | lineitem         | lineitem_pkey                      |            | CREATE UNIQUE INDEX lineitem_pkey ON lineitem USING btree (l_orderkey, l_linenumber)
+ public     | lineitem         | lineitem_pkey                      |            | CREATE UNIQUE INDEX lineitem_pkey ON public.lineitem USING btree (l_orderkey, l_linenumber)
- public     | lineitem         | lineitem_time_index                |            | CREATE INDEX lineitem_time_index ON lineitem USING btree (l_shipdate)
+ public     | lineitem         | lineitem_time_index                |            | CREATE INDEX lineitem_time_index ON public.lineitem USING btree (l_shipdate)
 (15 rows)
 
 \c - - - :worker_1_port
@@ -175,23 +175,23 @@ CREATE INDEX ON lineitem (l_orderkey);
 ERROR:  creating index without a name on a distributed table is currently unsupported
 -- Verify that none of failed indexes got created on the master node
 SELECT * FROM pg_indexes WHERE tablename = 'lineitem' or tablename like 'index_test_%' ORDER BY indexname;
- schemaname |    tablename     |             indexname              | tablespace |                                                      indexdef                                                       
+ schemaname |    tablename     |             indexname              | tablespace |                                                          indexdef                                                          
-------------+------------------+------------------------------------+------------+---------------------------------------------------------------------------------------------------------------------
+------------+------------------+------------------------------------+------------+----------------------------------------------------------------------------------------------------------------------------
- public     | index_test_hash  | index_test_hash_index_a            |            | CREATE UNIQUE INDEX index_test_hash_index_a ON index_test_hash USING btree (a)
+ public     | index_test_hash  | index_test_hash_index_a            |            | CREATE UNIQUE INDEX index_test_hash_index_a ON public.index_test_hash USING btree (a)
- public     | index_test_hash  | index_test_hash_index_a_b          |            | CREATE UNIQUE INDEX index_test_hash_index_a_b ON index_test_hash USING btree (a, b)
+ public     | index_test_hash  | index_test_hash_index_a_b          |            | CREATE UNIQUE INDEX index_test_hash_index_a_b ON public.index_test_hash USING btree (a, b)
- public     | index_test_hash  | index_test_hash_index_a_b_partial  |            | CREATE UNIQUE INDEX index_test_hash_index_a_b_partial ON index_test_hash USING btree (a, b) WHERE (c IS NOT NULL)
+ public     | index_test_hash  | index_test_hash_index_a_b_partial  |            | CREATE UNIQUE INDEX index_test_hash_index_a_b_partial ON public.index_test_hash USING btree (a, b) WHERE (c IS NOT NULL)
- public     | index_test_range | index_test_range_index_a           |            | CREATE UNIQUE INDEX index_test_range_index_a ON index_test_range USING btree (a)
+ public     | index_test_range | index_test_range_index_a           |            | CREATE UNIQUE INDEX index_test_range_index_a ON public.index_test_range USING btree (a)
- public     | index_test_range | index_test_range_index_a_b         |            | CREATE UNIQUE INDEX index_test_range_index_a_b ON index_test_range USING btree (a, b)
+ public     | index_test_range | index_test_range_index_a_b         |            | CREATE UNIQUE INDEX index_test_range_index_a_b ON public.index_test_range USING btree (a, b)
- public     | index_test_range | index_test_range_index_a_b_partial |            | CREATE UNIQUE INDEX index_test_range_index_a_b_partial ON index_test_range USING btree (a, b) WHERE (c IS NOT NULL)
+ public     | index_test_range | index_test_range_index_a_b_partial |            | CREATE UNIQUE INDEX index_test_range_index_a_b_partial ON public.index_test_range USING btree (a, b) WHERE (c IS NOT NULL)
- public     | lineitem         | lineitem_colref_index              |            | CREATE INDEX lineitem_colref_index ON lineitem USING btree (record_ne(lineitem.*, NULL::record))
+ public     | lineitem         | lineitem_colref_index              |            | CREATE INDEX lineitem_colref_index ON public.lineitem USING btree (record_ne(lineitem.*, NULL::record))
- public     | lineitem         | lineitem_concurrently_index        |            | CREATE INDEX lineitem_concurrently_index ON lineitem USING btree (l_orderkey)
+ public     | lineitem         | lineitem_concurrently_index        |            | CREATE INDEX lineitem_concurrently_index ON public.lineitem USING btree (l_orderkey)
- public     | lineitem         | lineitem_orderkey_hash_index       |            | CREATE INDEX lineitem_orderkey_hash_index ON lineitem USING hash (l_partkey)
+ public     | lineitem         | lineitem_orderkey_hash_index       |            | CREATE INDEX lineitem_orderkey_hash_index ON public.lineitem USING hash (l_partkey)
- public     | lineitem         | lineitem_orderkey_index            |            | CREATE INDEX lineitem_orderkey_index ON lineitem USING btree (l_orderkey)
+ public     | lineitem         | lineitem_orderkey_index            |            | CREATE INDEX lineitem_orderkey_index ON public.lineitem USING btree (l_orderkey)
- public     | lineitem         | lineitem_orderkey_index_new        |            | CREATE INDEX lineitem_orderkey_index_new ON lineitem USING btree (l_orderkey)
+ public     | lineitem         | lineitem_orderkey_index_new        |            | CREATE INDEX lineitem_orderkey_index_new ON public.lineitem USING btree (l_orderkey)
- public     | lineitem         | lineitem_partial_index             |            | CREATE INDEX lineitem_partial_index ON lineitem USING btree (l_shipdate) WHERE (l_shipdate < '01-01-1995'::date)
+ public     | lineitem         | lineitem_partial_index             |            | CREATE INDEX lineitem_partial_index ON public.lineitem USING btree (l_shipdate) WHERE (l_shipdate < '01-01-1995'::date)
- public     | lineitem         | lineitem_partkey_desc_index        |            | CREATE INDEX lineitem_partkey_desc_index ON lineitem USING btree (l_partkey DESC)
+ public     | lineitem         | lineitem_partkey_desc_index        |            | CREATE INDEX lineitem_partkey_desc_index ON public.lineitem USING btree (l_partkey DESC)
- public     | lineitem         | lineitem_pkey                      |            | CREATE UNIQUE INDEX lineitem_pkey ON lineitem USING btree (l_orderkey, l_linenumber)
+ public     | lineitem         | lineitem_pkey                      |            | CREATE UNIQUE INDEX lineitem_pkey ON public.lineitem USING btree (l_orderkey, l_linenumber)
- public     | lineitem         | lineitem_time_index                |            | CREATE INDEX lineitem_time_index ON lineitem USING btree (l_shipdate)
+ public     | lineitem         | lineitem_time_index                |            | CREATE INDEX lineitem_time_index ON public.lineitem USING btree (l_shipdate)
 (15 rows)
 
 --

src/test/regress/sql/multi_explain.sql

@@ -247,3 +247,11 @@ ALTER TABLE explain_table ADD COLUMN value int;
 EXPLAIN (COSTS FALSE) SELECT value FROM explain_table WHERE id = 1;
 
 ROLLBACK;
+
+-- Test disable hash aggregate
+SET enable_hashagg TO off;
+EXPLAIN (COSTS FALSE, FORMAT TEXT)
+	SELECT l_quantity, count(*) count_quantity FROM lineitem
+	GROUP BY l_quantity ORDER BY count_quantity, l_quantity;
+
+SET enable_hashagg TO on;