Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c19617d

Browse files
committed
In locate_grouping_columns(), don't expect an exact match of Var typmods.
It's possible that inlining of SQL functions (or perhaps other changes?) has exposed typmod information not known at parse time. In such cases, Vars generated by query_planner might have valid typmod values while the original grouping columns only have typmod -1. This isn't a semantic problem since the behavior of grouping only depends on type not typmod, but it breaks locate_grouping_columns' use of tlist_member to locate the matching entry in query_planner's result tlist. We can fix this without an excessive amount of new code or complexity by relying on the fact that locate_grouping_columns only gets called when make_subplanTargetList has set need_tlist_eval == false, and that can only happen if all the grouping columns are simple Vars. Therefore we only need to search the sub_tlist for a matching Var, and we can reasonably define a "match" as being a match of the Var identity fields varno/varattno/varlevelsup. The code still Asserts that vartype matches, but ignores vartypmod. Per bug #8393 from Evan Martin. The added regression test case is basically the same as his example. This has been broken for a very long time, so back-patch to all supported branches.
1 parent e6d3f5b commit c19617d

File tree

5 files changed

+66
-3
lines changed

5 files changed

+66
-3
lines changed

src/backend/optimizer/plan/planner.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2809,7 +2809,8 @@ choose_hashed_distinct(PlannerInfo *root,
28092809
* 'groupColIdx' receives an array of column numbers for the GROUP BY
28102810
* expressions (if there are any) in the returned target list.
28112811
* 'need_tlist_eval' is set true if we really need to evaluate the
2812-
* returned tlist as-is.
2812+
* returned tlist as-is. (Note: locate_grouping_columns assumes
2813+
* that if this is FALSE, all grouping columns are simple Vars.)
28132814
*
28142815
* The result is the targetlist to be passed to query_planner.
28152816
*/
@@ -2972,6 +2973,7 @@ get_grouping_column_index(Query *parse, TargetEntry *tle)
29722973
* This is only needed if we don't use the sub_tlist chosen by
29732974
* make_subplanTargetList. We have to forget the column indexes found
29742975
* by that routine and re-locate the grouping exprs in the real sub_tlist.
2976+
* We assume the grouping exprs are just Vars (see make_subplanTargetList).
29752977
*/
29762978
static void
29772979
locate_grouping_columns(PlannerInfo *root,
@@ -2995,11 +2997,24 @@ locate_grouping_columns(PlannerInfo *root,
29952997
foreach(gl, root->parse->groupClause)
29962998
{
29972999
SortGroupClause *grpcl = (SortGroupClause *) lfirst(gl);
2998-
Node *groupexpr = get_sortgroupclause_expr(grpcl, tlist);
2999-
TargetEntry *te = tlist_member(groupexpr, sub_tlist);
3000+
Var *groupexpr = (Var *) get_sortgroupclause_expr(grpcl, tlist);
3001+
TargetEntry *te;
30003002

3003+
/*
3004+
* The grouping column returned by create_plan might not have the same
3005+
* typmod as the original Var. (This can happen in cases where a
3006+
* set-returning function has been inlined, so that we now have more
3007+
* knowledge about what it returns than we did when the original Var
3008+
* was created.) So we can't use tlist_member() to search the tlist;
3009+
* instead use tlist_member_match_var. For safety, still check that
3010+
* the vartype matches.
3011+
*/
3012+
if (!(groupexpr && IsA(groupexpr, Var)))
3013+
elog(ERROR, "grouping column is not a Var as expected");
3014+
te = tlist_member_match_var(groupexpr, sub_tlist);
30013015
if (!te)
30023016
elog(ERROR, "failed to locate grouping columns");
3017+
Assert(((Var *) te->expr)->vartype == groupexpr->vartype);
30033018
groupColIdx[keyno++] = te->resno;
30043019
}
30053020
}

src/backend/optimizer/util/tlist.c

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,35 @@ tlist_member_ignore_relabel(Node *node, List *targetlist)
7171
return NULL;
7272
}
7373

74+
/*
75+
* tlist_member_match_var
76+
* Same as above, except that we match the provided Var on the basis
77+
* of varno/varattno/varlevelsup only, rather than using full equal().
78+
*
79+
* This is needed in some cases where we can't be sure of an exact typmod
80+
* match. It's probably a good idea to check the vartype anyway, but
81+
* we leave it to the caller to apply any suitable sanity checks.
82+
*/
83+
TargetEntry *
84+
tlist_member_match_var(Var *var, List *targetlist)
85+
{
86+
ListCell *temp;
87+
88+
foreach(temp, targetlist)
89+
{
90+
TargetEntry *tlentry = (TargetEntry *) lfirst(temp);
91+
Var *tlvar = (Var *) tlentry->expr;
92+
93+
if (!tlvar || !IsA(tlvar, Var))
94+
continue;
95+
if (var->varno == tlvar->varno &&
96+
var->varattno == tlvar->varattno &&
97+
var->varlevelsup == tlvar->varlevelsup)
98+
return tlentry;
99+
}
100+
return NULL;
101+
}
102+
74103
/*
75104
* flatten_tlist
76105
* Create a target list that only contains unique variables.

src/include/optimizer/tlist.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
extern TargetEntry *tlist_member(Node *node, List *targetlist);
2121
extern TargetEntry *tlist_member_ignore_relabel(Node *node, List *targetlist);
22+
extern TargetEntry *tlist_member_match_var(Var *var, List *targetlist);
2223

2324
extern List *flatten_tlist(List *tlist, PVCAggregateBehavior aggbehavior,
2425
PVCPlaceHolderBehavior phbehavior);

src/test/regress/expected/rangefuncs.out

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,17 @@ SELECT * FROM foo(3);
576576
(9 rows)
577577

578578
DROP FUNCTION foo(int);
579+
-- case that causes change of typmod knowledge during inlining
580+
CREATE OR REPLACE FUNCTION foo()
581+
RETURNS TABLE(a varchar(5))
582+
AS $$ SELECT 'hello'::varchar(5) $$ LANGUAGE sql STABLE;
583+
SELECT * FROM foo() GROUP BY 1;
584+
a
585+
-------
586+
hello
587+
(1 row)
588+
589+
DROP FUNCTION foo();
579590
--
580591
-- some tests on SQL functions with RETURNING
581592
--

src/test/regress/sql/rangefuncs.sql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,13 @@ AS $$ SELECT a, b
286286
SELECT * FROM foo(3);
287287
DROP FUNCTION foo(int);
288288

289+
-- case that causes change of typmod knowledge during inlining
290+
CREATE OR REPLACE FUNCTION foo()
291+
RETURNS TABLE(a varchar(5))
292+
AS $$ SELECT 'hello'::varchar(5) $$ LANGUAGE sql STABLE;
293+
SELECT * FROM foo() GROUP BY 1;
294+
DROP FUNCTION foo();
295+
289296
--
290297
-- some tests on SQL functions with RETURNING
291298
--

0 commit comments

Comments
 (0)