From 917cb6ae933d2364d1af35dc7e4126528aaca11d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Wed, 25 Mar 2020 13:52:57 +0000 Subject: [PATCH] Don't segfault on queries using GROUPING GROUPING will always return 0 outside of GROUPING SETS, CUBE, or ROLLUP Since we don't support those, it makes sense to reject GROUPING in queries --- .../planner/extended_op_node_utils.c | 5 ++--- .../planner/multi_logical_planner.c | 18 ++++++++++++++++++ .../planner/multi_physical_planner.c | 2 +- .../regress/expected/aggregate_support.out | 15 +++++++++++++++ src/test/regress/sql/aggregate_support.sql | 13 +++++++++++++ 5 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/planner/extended_op_node_utils.c b/src/backend/distributed/planner/extended_op_node_utils.c index 6078c0f17..8428a5ae8 100644 --- a/src/backend/distributed/planner/extended_op_node_utils.c +++ b/src/backend/distributed/planner/extended_op_node_utils.c @@ -225,13 +225,12 @@ HasNonPartitionColumnDistinctAgg(List *targetEntryList, Node *havingQual, ListCell *varCell = NULL; bool isPartitionColumn = false; - if (IsA(targetNode, Var)) + if (!IsA(targetNode, Aggref)) { continue; } - Assert(IsA(targetNode, Aggref)); - Aggref *targetAgg = (Aggref *) targetNode; + Aggref *targetAgg = castNode(Aggref, targetNode); if (targetAgg->aggdistinct == NIL) { continue; diff --git a/src/backend/distributed/planner/multi_logical_planner.c b/src/backend/distributed/planner/multi_logical_planner.c index d022dd0ef..0448bd995 100644 --- a/src/backend/distributed/planner/multi_logical_planner.c +++ b/src/backend/distributed/planner/multi_logical_planner.c @@ -82,6 +82,7 @@ static bool IsReadIntermediateResultFunction(Node *node); static bool IsReadIntermediateResultArrayFunction(Node *node); static bool IsCitusExtraDataContainerFunc(Node *node); static bool IsFunctionWithOid(Node *node, Oid funcOid); +static bool IsGroupingFunc(Node *node); static bool ExtractFromExpressionWalker(Node *node, QualifierWalkerContext *walkerContext); static List * MultiTableNodeList(List *tableEntryList, List *rangeTableList); @@ -884,6 +885,16 @@ IsFunctionWithOid(Node *node, Oid funcOid) } +/* + * IsGroupingFunc returns whether node is a GroupingFunc. + */ +static bool +IsGroupingFunc(Node *node) +{ + return IsA(node, GroupingFunc); +} + + /* * FindIntermediateResultIdIfExists extracts the id of the intermediate result * if the given RTE contains a read_intermediate_results function, NULL otherwise @@ -978,6 +989,13 @@ DeferErrorIfQueryNotSupported(Query *queryTree) errorHint = filterHint; } + if (FindNodeCheck((Node *) queryTree, IsGroupingFunc)) + { + preconditionsSatisfied = false; + errorMessage = "could not run distributed query with GROUPING"; + errorHint = filterHint; + } + bool hasTablesample = HasTablesample(queryTree); if (hasTablesample) { diff --git a/src/backend/distributed/planner/multi_physical_planner.c b/src/backend/distributed/planner/multi_physical_planner.c index d94eb1ab9..9b9a9de7d 100644 --- a/src/backend/distributed/planner/multi_physical_planner.c +++ b/src/backend/distributed/planner/multi_physical_planner.c @@ -1017,7 +1017,7 @@ AddAnyValueAggregates(Node *node, void *context) } } } - if (IsA(node, Aggref)) + if (IsA(node, Aggref) || IsA(node, GroupingFunc)) { return node; } diff --git a/src/test/regress/expected/aggregate_support.out b/src/test/regress/expected/aggregate_support.out index e8cc08593..bc19f96cd 100644 --- a/src/test/regress/expected/aggregate_support.out +++ b/src/test/regress/expected/aggregate_support.out @@ -432,6 +432,21 @@ select key, count(distinct aggdata) from aggdata group by key order by 1, 2; ERROR: type "aggregate_support.aggdata" does not exist CONTEXT: while executing command on localhost:xxxxx +-- GROUPING parses to GroupingFunc, distinct from Aggref +-- These three queries represent edge cases implementation would have to consider +-- For now we error out of all three +select grouping(id) +from aggdata group by id order by 1 limit 3; +ERROR: could not run distributed query with GROUPING +HINT: Consider using an equality filter on the distributed table's partition column. +select key, grouping(val) +from aggdata group by key, val order by 1, 2; +ERROR: could not run distributed query with GROUPING +HINT: Consider using an equality filter on the distributed table's partition column. +select key, grouping(val), sum(distinct valf) +from aggdata group by key, val order by 1, 2; +ERROR: could not run distributed query with GROUPING +HINT: Consider using an equality filter on the distributed table's partition column. -- Test https://github.com/citusdata/citus/issues/3328 create table nulltable(id int); insert into nulltable values (0); diff --git a/src/test/regress/sql/aggregate_support.sql b/src/test/regress/sql/aggregate_support.sql index faec5f612..7bde84200 100644 --- a/src/test/regress/sql/aggregate_support.sql +++ b/src/test/regress/sql/aggregate_support.sql @@ -204,6 +204,19 @@ RESET citus.task_executor_type; select key, count(distinct aggdata) from aggdata group by key order by 1, 2; +-- GROUPING parses to GroupingFunc, distinct from Aggref +-- These three queries represent edge cases implementation would have to consider +-- For now we error out of all three +select grouping(id) +from aggdata group by id order by 1 limit 3; + +select key, grouping(val) +from aggdata group by key, val order by 1, 2; + +select key, grouping(val), sum(distinct valf) +from aggdata group by key, val order by 1, 2; + + -- Test https://github.com/citusdata/citus/issues/3328 create table nulltable(id int); insert into nulltable values (0);