Thread: BUG #5240: Stable Functions that return a table type with a dropped column fail

The following bug has been logged online:

Bug reference:      5240
Logged by:          David Gardner
Email address:      dgardner@creatureshop.com
PostgreSQL version: 8.4.1
Operating system:   Debian Linux, amd64 2.6.30
Description:        Stable Functions that return a table type with a dropped
column fail
Details:

SELECT foo(); works while SELECT * FROM foo(); fails.
However redefining the function as volatile fixes the issue. Possibly
related to BUG #4907.

-----------------------
dgardner@cssun32 ~:$ psql -h localhost -d hdpsdb_baseline
Password:
psql (8.4.1)
Type "help" for help.

hdpsdb_baseline=# SELECT * FROM version();
                                               version

----------------------------------------------------------------------------
--------------------------
 PostgreSQL 8.4.1 on x86_64-pc-linux-gnu, compiled by GCC gcc-4.3.real
(Debian 4.3.4-2) 4.3.4, 64-bit
(1 row)

hdpsdb_baseline=# SELECT a.attnum, a.attname AS field,
hdpsdb_baseline-#        a.attlen AS length, a.atttypmod AS length_var,
hdpsdb_baseline-#        a.attnotnull AS not_null, a.atthasdef as
has_default
hdpsdb_baseline-#   FROM pg_class c, pg_attribute a
hdpsdb_baseline-#  WHERE c.relname = 'users'
hdpsdb_baseline-#    AND a.attnum > 0
hdpsdb_baseline-#    AND a.attrelid = c.oid
hdpsdb_baseline-#  ORDER BY a.attnum;
 attnum |            field             | length | length_var | not_null |
has_default
--------+------------------------------+--------+------------+----------+---
----------
      1 | userid                       |     -1 |        259 | t        | f
      2 | name                         |     -1 |        259 | t        | f
      3 | email                        |     -1 |         -1 | f        | f
      4 | ........pg.dropped.4........ |      1 |         -1 | f        | f
      5 | enabled                      |      1 |         -1 | t        | t
(5 rows)

hdpsdb_baseline=# CREATE OR REPLACE FUNCTION get_users()
hdpsdb_baseline-#   RETURNS SETOF users AS
hdpsdb_baseline-# $BODY$
hdpsdb_baseline$# SELECT users.*
hdpsdb_baseline$# FROM
hdpsdb_baseline$# users
hdpsdb_baseline$# ORDER BY users.userid LIMIT 1;
hdpsdb_baseline$# $BODY$
hdpsdb_baseline-#   LANGUAGE 'sql' STABLE;
CREATE FUNCTION
hdpsdb_baseline=# ALTER FUNCTION get_users() OWNER TO dgardner;
ALTER FUNCTION
hdpsdb_baseline=# GRANT EXECUTE ON FUNCTION get_users() TO public;
GRANT
hdpsdb_baseline=# GRANT EXECUTE ON FUNCTION get_users() TO dgardner;
GRANT
hdpsdb_baseline=#
hdpsdb_baseline=# SELECT * FROM get_users();
ERROR:  invalid attribute number 5
hdpsdb_baseline=# SELECT get_users();
               get_users
---------------------------------------
 (aa_test_user,test,test@here.there,t)
(1 row)

hdpsdb_baseline=# CREATE OR REPLACE FUNCTION get_users()
hdpsdb_baseline-#   RETURNS SETOF users AS
hdpsdb_baseline-# $BODY$
hdpsdb_baseline$# SELECT users.*
hdpsdb_baseline$# FROM
hdpsdb_baseline$# users
hdpsdb_baseline$# ORDER BY users.userid LIMIT 1;
hdpsdb_baseline$# $BODY$
hdpsdb_baseline-#   LANGUAGE 'sql' VOLATILE;
CREATE FUNCTION
hdpsdb_baseline=# ALTER FUNCTION get_users() OWNER TO dgardner;
ALTER FUNCTION
hdpsdb_baseline=# GRANT EXECUTE ON FUNCTION get_users() TO public;
GRANT
hdpsdb_baseline=# GRANT EXECUTE ON FUNCTION get_users() TO dgardner;
GRANT
hdpsdb_baseline=# SELECT * FROM get_users();
    userid    | name |      email      | enabled
--------------+------+-----------------+---------
 aa_test_user | test | test@here.there | t
(1 row)

hdpsdb_baseline=#

>>>>> "David" == "David Gardner" <dgardner@creatureshop.com> writes:

 David> The following bug has been logged online:

 David> Bug reference:      5240
 David> Logged by:          David Gardner
 David> Email address:      dgardner@creatureshop.com
 David> PostgreSQL version: 8.4.1
 David> Operating system:   Debian Linux, amd64 2.6.30
 David> Description:        Stable Functions that return a table type with a dropped
 David> column fail
 David> Details:

 David> SELECT foo(); works while SELECT * FROM foo(); fails.
 David> However redefining the function as volatile fixes the
 David> issue. Possibly related to BUG #4907.

I don't think it's particularly closely related to #4907.

I've confirmed this bug still exists in both 8.4.2 and HEAD; it's
clearly a problem that affects only inlined SQL functions (making the
function volatile defeats inlining, and thus avoids the bug).

--
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> I don't think it's particularly closely related to #4907.

Yeah.  I think the real problem is that set_subqueryscan_references
is overly optimistic about how much work it has to do:

         * ...  Notice we do
         * not do set_upper_references() here, because a SubqueryScan will
         * always have been created with correct references to its subplan's
         * outputs to begin with.

The upper query will have Vars with attnos that correspond to the
function's output rowtype, ie, 1,2,3,5 in this example.  However the
actual output of the lower query only has 4 columns.  Kinda surprising
we never hit this before, really, although I guess there are not that
many cases where a subquery could have a named rowtype.

            regards, tom lane

[ just a note for the archives ]

I wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>> I don't think it's particularly closely related to #4907.

> Yeah.  I think the real problem is that set_subqueryscan_references
> is overly optimistic about how much work it has to do:

On further review it doesn't seem that fixing it there is very
practical.  Currently, the Vars in the SubqueryScan's tlist and quals
have varno referencing the subquery RTE and attno matching the column
numbers of the function's declared result type.  We can't just renumber
the attnos to match the actual subquery outputs without confusing the
heck out of EXPLAIN.  It would be okay to change the attnos to match
if we also changed the varno to be OUTER --- then EXPLAIN would
understand the Vars as being references to the subplan's tlist.  However
that would break the executor, in particular it would mean that
nodeSubqueryScan couldn't use the execScan.c facilities, since execScan
expects the expressions it's dealing with to reference the scan tuple
slot not the outer tuple slot.

I'm currently thinking that the best fix is to make inline expansion of
a SRF insert NULL columns into the expanded function tlist so that the
column numbering still matches up.  This probably means that we'd not be
able to inline in certain cases, like if the SRF contains a UNION or
other operation where modifying the tlist changes the semantics.

            regards, tom lane

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 > Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
 >> I don't think it's particularly closely related to #4907.

 Tom> Yeah.

BTW, did #4907 ever get fixed in the back branches?

--
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> BTW, did #4907 ever get fixed in the back branches?

No, I didn't think it was prudent to back-patch it.  Also, #5240
represents an actual regression (cases that worked before 8.4 now
fail) whereas the plpgsql cases never have worked.

            regards, tom lane