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.

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 789af4e9f9853e98cf482a2141993ef264a403e3..559458a7191f8e103bbc6620e9297e83bd7745f0 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -2679,7 +2679,8 @@ choose_hashed_distinct(PlannerInfo *root,
  * 'groupColIdx' receives an array of column numbers for the GROUP BY
  *                     expressions (if there are any) in the returned target list.
  * 'need_tlist_eval' is set true if we really need to evaluate the
- *                     returned tlist as-is.
+ *                     returned tlist as-is.  (Note: locate_grouping_columns assumes
+ *                     that if this is FALSE, all grouping columns are simple Vars.)
  *
  * The result is the targetlist to be passed to query_planner.
  */
@@ -2842,6 +2843,7 @@ get_grouping_column_index(Query *parse, TargetEntry *tle)
  * This is only needed if we don't use the sub_tlist chosen by
  * make_subplanTargetList.     We have to forget the column indexes found
  * by that routine and re-locate the grouping exprs in the real sub_tlist.
+ * We assume the grouping exprs are just Vars (see make_subplanTargetList).
  */
 static void
 locate_grouping_columns(PlannerInfo *root,
@@ -2865,11 +2867,24 @@ locate_grouping_columns(PlannerInfo *root,
        foreach(gl, root->parse->groupClause)
        {
                SortGroupClause *grpcl = (SortGroupClause *) lfirst(gl);
-               Node       *groupexpr = get_sortgroupclause_expr(grpcl, tlist);
-               TargetEntry *te = tlist_member(groupexpr, sub_tlist);
+               Var                *groupexpr = (Var *) get_sortgroupclause_expr(grpcl, tlist);
+               TargetEntry *te;
 
+               /*
+                * The grouping column returned by create_plan might not have the same
+                * typmod as the original Var.  (This can happen in cases where a
+                * set-returning function has been inlined, so that we now have more
+                * knowledge about what it returns than we did when the original Var
+                * was created.)  So we can't use tlist_member() to search the tlist;
+                * instead use tlist_member_match_var.  For safety, still check that
+                * the vartype matches.
+                */
+               if (!(groupexpr && IsA(groupexpr, Var)))
+                       elog(ERROR, "grouping column is not a Var as expected");
+               te = tlist_member_match_var(groupexpr, sub_tlist);
                if (!te)
                        elog(ERROR, "failed to locate grouping columns");
+               Assert(((Var *) te->expr)->vartype == groupexpr->vartype);
                groupColIdx[keyno++] = te->resno;
        }
 }

diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 3031c404f579ffd2ed680ea0c517388e515a4344..5c80bccbb29e5b179c40285cae64dd86e140cc46 100644 (file)
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -71,6 +71,35 @@ tlist_member_ignore_relabel(Node *node, List *targetlist)
        return NULL;
 }
 
+/*
+ * tlist_member_match_var
+ *       Same as above, except that we match the provided Var on the basis
+ *       of varno/varattno/varlevelsup only, rather than using full equal().
+ *
+ * This is needed in some cases where we can't be sure of an exact typmod
+ * match.  It's probably a good idea to check the vartype anyway, but
+ * we leave it to the caller to apply any suitable sanity checks.
+ */
+TargetEntry *
+tlist_member_match_var(Var *var, List *targetlist)
+{
+       ListCell   *temp;
+
+       foreach(temp, targetlist)
+       {
+               TargetEntry *tlentry = (TargetEntry *) lfirst(temp);
+               Var                *tlvar = (Var *) tlentry->expr;
+
+               if (!tlvar || !IsA(tlvar, Var))
+                       continue;
+               if (var->varno == tlvar->varno &&
+                       var->varattno == tlvar->varattno &&
+                       var->varlevelsup == tlvar->varlevelsup)
+                       return tlentry;
+       }
+       return NULL;
+}
+
 /*
  * flatten_tlist
  *       Create a target list that only contains unique variables.

diff --git a/src/include/optimizer/tlist.h b/src/include/optimizer/tlist.h
index 693d58c6a1bbf5cf47e307af64ab03be26b7e23c..01bec54dba49aafc906ea226da9f5e33691e6f0d 100644 (file)
--- a/src/include/optimizer/tlist.h
+++ b/src/include/optimizer/tlist.h
@@ -19,6 +19,7 @@
 
 extern TargetEntry *tlist_member(Node *node, List *targetlist);
 extern TargetEntry *tlist_member_ignore_relabel(Node *node, List *targetlist);
+extern TargetEntry *tlist_member_match_var(Var *var, List *targetlist);
 
 extern List *flatten_tlist(List *tlist, PVCAggregateBehavior aggbehavior,
                          PVCPlaceHolderBehavior phbehavior);

diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 5f20c9324975eda8e52c730f621a76edf708a4bf..7a2c81caf17edef299a17134b1f6d8ac39db1c0e 100644 (file)
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -575,6 +575,17 @@ SELECT * FROM foo(3);
 (9 rows)
 
 DROP FUNCTION foo(int);
+-- case that causes change of typmod knowledge during inlining
+CREATE OR REPLACE FUNCTION foo()
+RETURNS TABLE(a varchar(5))
+AS $$ SELECT 'hello'::varchar(5) $$ LANGUAGE sql STABLE;
+SELECT * FROM foo() GROUP BY 1;
+   a   
+-------
+ hello
+(1 row)
+
+DROP FUNCTION foo();
 --
 -- some tests on SQL functions with RETURNING
 --

diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index 54cfc178c057c40f82e27cbd1f3a8e0e271c34b3..9a333c266ea5a9f1c0ff2c03bfa83f083f1b144b 100644 (file)
--- a/src/test/regress/sql/rangefuncs.sql
+++ b/src/test/regress/sql/rangefuncs.sql
@@ -286,6 +286,13 @@ AS $$ SELECT a, b
 SELECT * FROM foo(3);
 DROP FUNCTION foo(int);
 
+-- case that causes change of typmod knowledge during inlining
+CREATE OR REPLACE FUNCTION foo()
+RETURNS TABLE(a varchar(5))
+AS $$ SELECT 'hello'::varchar(5) $$ LANGUAGE sql STABLE;
+SELECT * FROM foo() GROUP BY 1;
+DROP FUNCTION foo();
+
 --
 -- some tests on SQL functions with RETURNING
 --