From 785a596df16b9b7f898bd7d93d0297f6501fff27 Mon Sep 17 00:00:00 2001 From: chenxiaobin19 <1025221611@qq.com> Date: Mon, 26 Dec 2022 17:50:46 +0800 Subject: [PATCH] agg cannot contain set-returning function calls --- src/common/backend/parser/parse_agg.cpp | 8 +++ .../optimizer/rewrite/rewriteManip.cpp | 72 ++++++++++++++++--- src/include/rewrite/rewriteManip.h | 2 + src/test/regress/expected/agg.out | 28 ++++---- src/test/regress/sql/agg.sql | 3 + src/tools/pgindent/typedefs.list | 2 +- 6 files changed, 93 insertions(+), 22 deletions(-) diff --git a/src/common/backend/parser/parse_agg.cpp b/src/common/backend/parser/parse_agg.cpp index a722fa4ed5..602653844f 100644 --- a/src/common/backend/parser/parse_agg.cpp +++ b/src/common/backend/parser/parse_agg.cpp @@ -211,6 +211,14 @@ void transformAggregateCall(ParseState* pstate, Aggref* agg, List* args, List* a } } + /* It can't contain set-returning functions either */ + if (checkExprHasSetReturningFuncs((Node*)agg->args)) { + ereport(ERROR, + (errcode(ERRCODE_GROUPING_ERROR), + errmsg("aggregate function calls cannot contain set-returning function calls"), + parser_errposition(pstate, locate_srfunc((Node*)agg->args)))); + } + /* It can't contain window functions either */ if (pstate->p_hasWindowFuncs && checkExprHasWindowFuncs((Node*)agg->args)) { ereport(ERROR, diff --git a/src/gausskernel/optimizer/rewrite/rewriteManip.cpp b/src/gausskernel/optimizer/rewrite/rewriteManip.cpp index cd2776494a..9f3eb324a7 100644 --- a/src/gausskernel/optimizer/rewrite/rewriteManip.cpp +++ b/src/gausskernel/optimizer/rewrite/rewriteManip.cpp @@ -36,14 +36,16 @@ typedef struct { } locate_agg_of_level_context; typedef struct { - int win_location; -} locate_windowfunc_context; + int location; +} locate_func_context; static bool contain_aggs_of_level_or_above_walker(Node* node, int* sublevels_up); static bool contain_aggs_of_level_walker(Node* node, contain_aggs_of_level_context* context); static bool locate_agg_of_level_walker(Node* node, locate_agg_of_level_context* context); +static bool contain_srfunc_walker(Node* node, void* context); +static bool locate_srfunc_walker(Node* node, locate_func_context* context); static bool contain_windowfuncs_walker(Node* node, void* context); -static bool locate_windowfunc_walker(Node* node, locate_windowfunc_context* context); +static bool locate_windowfunc_walker(Node* node, locate_func_context* context); static bool checkExprHasSubLink_walker(Node* node, void* context); static Relids offset_relid_set(Relids relids, int offset); static Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid); @@ -210,6 +212,60 @@ static bool locate_agg_of_level_walker(Node* node, locate_agg_of_level_context* return expression_tree_walker(node, (bool (*)())locate_agg_of_level_walker, (void*)context); } +/* + * checkExprHasSetReturningFuncs - + * Check if an expression contains a set-returning function call of the + * current query level. + */ +bool checkExprHasSetReturningFuncs(Node* node) +{ + /* + * Must be prepared to start with a Query or a bare expression tree; if + * it's a Query, we don't want to increment sublevels_up. + */ + return query_or_expression_tree_walker(node, (bool (*)())contain_srfunc_walker, NULL, 0); +} + +static bool contain_srfunc_walker(Node* node, void* context) +{ + if (node == NULL) + return false; + if (IsA(node, FuncExpr) && ((FuncExpr*)node)->funcretset) + return true; /* abort the tree traversal and return true */ + /* Mustn't recurse into subselects */ + return expression_tree_walker(node, (bool (*)())contain_srfunc_walker, (void*)context); +} + +int locate_srfunc(Node* node) +{ + locate_func_context context; + + context.location = -1; /* in case we find nothing */ + + /* + * Must be prepared to start with a Query or a bare expression tree; if + * it's a Query, we don't want to increment sublevels_up. + */ + (void)query_or_expression_tree_walker(node, (bool (*)())locate_srfunc_walker, (void*)&context, 0); + + return context.location; +} + +static bool locate_srfunc_walker(Node* node, locate_func_context* context) +{ + if (node == NULL) + return false; + if (IsA(node, FuncExpr) && ((FuncExpr*)node)->funcretset) { + if (((FuncExpr*)node)->location >= 0) { + context->location = ((FuncExpr*)node)->location; + return true; /* abort the tree traversal and return true */ + } + /* else fall through to examine argument */ + } + /* Mustn't recurse into subselects */ + return expression_tree_walker(node, (bool (*)())locate_srfunc_walker, (void*)context); +} + /* * checkExprHasWindowFuncs - * Check if an expression contains a window function call of the @@ -249,9 +305,9 @@ static bool contain_windowfuncs_walker(Node* node, void* context) */ int locate_windowfunc(Node* node) { - locate_windowfunc_context context; + locate_func_context context; - context.win_location = -1; /* in case we find nothing */ + context.location = -1; /* in case we find nothing */ /* * Must be prepared to start with a Query or a bare expression tree; if @@ -259,16 +315,16 @@ int locate_windowfunc(Node* node) */ (void)query_or_expression_tree_walker(node, (bool (*)())locate_windowfunc_walker, (void*)&context, 0); - return context.win_location; + return context.location; } -static bool locate_windowfunc_walker(Node* node, locate_windowfunc_context* context) +static bool locate_windowfunc_walker(Node* node, locate_func_context* context) { if (node == NULL) return false; if (IsA(node, WindowFunc)) { if (((WindowFunc*)node)->location >= 0) { - context->win_location = ((WindowFunc*)node)->location; + context->location = ((WindowFunc*)node)->location; return true; /* abort the tree traversal and return true */ } /* else fall through to examine argument */ diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h index c687135b6d..68026101fa 100644 --- a/src/include/rewrite/rewriteManip.h +++ b/src/include/rewrite/rewriteManip.h @@ -50,8 +50,10 @@ extern void AddInvertedQual(Query* parsetree, Node* qual); extern bool contain_aggs_of_level_or_above(Node* node, int levelsup); extern bool contain_aggs_of_level(Node* node, int levelsup); extern int locate_agg_of_level(Node* node, int levelsup); +extern int locate_srfunc(Node* node); extern int locate_windowfunc(Node* node); extern bool checkExprHasAggs(Node* node); +extern bool checkExprHasSetReturningFuncs(Node* node); extern bool checkExprHasWindowFuncs(Node* node); extern bool checkExprHasSubLink(Node* node); diff --git a/src/test/regress/expected/agg.out b/src/test/regress/expected/agg.out index cccdef6f64..a2a237bc24 100644 --- a/src/test/regress/expected/agg.out +++ b/src/test/regress/expected/agg.out @@ -5,21 +5,23 @@ insert into t1 values(1,2); set enable_hashagg = off; --force hash agg, if used sort agg will report error. select a , count(distinct generate_series(1,2)) from t1 group by a; -ERROR: set-valued function called in context when calculate targetlist that cannot accept a set +ERROR: aggregate function calls cannot contain set-returning function calls +LINE 1: select a , count(distinct generate_series(1,2)) from t1 gro... + ^ +CONTEXT: referenced column: count explain (verbose, costs off) select a , count(distinct generate_series(1,2)) from t1 group by a; - QUERY PLAN ----------------------------------------------------- - GroupAggregate - Output: a, count(DISTINCT generate_series(1, 2)) - Group By Key: t1.a - -> Sort - Output: a - Sort Key: t1.a - -> Seq Scan on aggregate.t1 - Output: a -(8 rows) - +ERROR: aggregate function calls cannot contain set-returning function calls +LINE 2: select a , count(distinct generate_series(1,2)) from t1 gro... + ^ +CONTEXT: referenced column: count +set query_dop = 2; +select a , count(distinct generate_series(1,2)) from t1 group by a; +ERROR: aggregate function calls cannot contain set-returning function calls +LINE 1: select a , count(distinct generate_series(1,2)) from t1 gro... + ^ +CONTEXT: referenced column: count +reset query_dop; --test const-false agg CREATE TABLE bmsql_item ( i_id int4 NoT NULL,i_name varchar(24),i_price numeric(5,2),i_data varchar( 50),i_im_id int4, diff --git a/src/test/regress/sql/agg.sql b/src/test/regress/sql/agg.sql index 2e2aa7206c..7c4d1767b6 100644 --- a/src/test/regress/sql/agg.sql +++ b/src/test/regress/sql/agg.sql @@ -8,6 +8,9 @@ set enable_hashagg = off; select a , count(distinct generate_series(1,2)) from t1 group by a; explain (verbose, costs off) select a , count(distinct generate_series(1,2)) from t1 group by a; +set query_dop = 2; +select a , count(distinct generate_series(1,2)) from t1 group by a; +reset query_dop; --test const-false agg CREATE TABLE bmsql_item ( diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index e7729e1de7..64ba198422 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2066,7 +2066,7 @@ locale_t locate_agg_of_level_context locate_var_of_level_context locate_var_of_relation_context -locate_windowfunc_context +locate_func_context logstreamer_param lquery lquery_level -- Gitee