FAQ
pl_comp.c does this to set up Params representing values it supplies for
expression/query execution:

param = makeNode(Param);
param->paramkind = PARAM_EXTERN;
param->paramid = dno + 1;
param->paramtype = exec_get_datum_type(estate, datum);
param->paramtypmod = -1;
param->paramcollid = exec_get_datum_collation(estate, datum);
param->location = location;

On reflection it seems a bit silly to not supply typmod: we know a
typmod for each plpgsql variable or record field, so why not provide it?

This omission is the cause of bug #6020:
http://archives.postgresql.org/pgsql-bugs/2011-05/msg00080.php

The submitter's example can be boiled down to this self-contained case:

CREATE OR REPLACE FUNCTION add_new_card()
RETURNS record AS
$BODY$
DECLARE
new_card_id INTEGER;
magic_byte CHAR(8);
crazy_eights CHAR(8);
return_pieces RECORD;
BEGIN
new_card_id := 42;
magic_byte := '12345678';
crazy_eights := 'abcd1234';

return_pieces := (new_card_id, magic_byte, crazy_eights);
RETURN return_pieces;
END
$BODY$
LANGUAGE plpgsql;

SELECT * FROM
add_new_card() AS (id INTEGER, magic CHAR(8), crazy CHAR(8));

Note that the reason this code broke in 9.0 is not that 9.0 got dumber
about supplying typmod data for plpgsql variables --- we never did that
before, which is why the Params are being set up without it. Rather,
it's that 9.0 is actually checking for typmod match where previous
releases didn't. If you increase the length of magic_byte in the above
example, pre-9.0 happily returns data that violates the length
constraint shown in the outer query.

Another consideration here is that because I back-ported the 9.0
checking code into REL8_4:
http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=5d3853a7fa40b28b44b14084863fd83a188c9a9e
8.4.8 in fact behaves like 9.0, which would be a regression for somebody
who relies on code like this to work.

I think the appropriate fix is pretty clear: add a function similar to
exec_get_datum_type that returns the datum's typmod, and use that to set
paramtypmod properly. What is worrying me is that it's not clear how
much user-visible behavioral change will result, and therefore I'm not
entirely comfortable with the idea of back-patching such a change into
9.0 --- and it wouldn't work at all in 8.4, where there simply isn't a
good opportunity to set a typmod for parameters passed to the main
executor (since the SPI interfaces plpgsql uses don't support that).

I don't especially want to revert the above-mentioned 8.4 commit
altogether, but maybe we should consider tweaking its version of
tupconvert.c to not check typmods? All the alternatives there seem
unpalatable.

Any opinions what to do?

regards, tom lane

Search Discussions

  • Tom Lane at May 12, 2011 at 9:55 pm

    I wrote:
    I think the appropriate fix is pretty clear: add a function similar to
    exec_get_datum_type that returns the datum's typmod, and use that to set
    paramtypmod properly. What is worrying me is that it's not clear how
    much user-visible behavioral change will result, and therefore I'm not
    entirely comfortable with the idea of back-patching such a change into
    9.0 --- and it wouldn't work at all in 8.4, where there simply isn't a
    good opportunity to set a typmod for parameters passed to the main
    executor (since the SPI interfaces plpgsql uses don't support that).
    Attached is a proposed patch for HEAD that sets up the Param's typmod
    sanely. I've verified that this fixes the reported problem and does not
    result in any changes in the regression tests, which makes me a bit more
    optimistic about it ... but I'm still not convinced it'd be a good idea
    to back-patch into 9.0.

    regards, tom lane
  • Robert Haas at May 14, 2011 at 2:17 am

    On Thu, May 12, 2011 at 5:55 PM, Tom Lane wrote:
    I wrote:
    I think the appropriate fix is pretty clear: add a function similar to
    exec_get_datum_type that returns the datum's typmod, and use that to set
    paramtypmod properly.  What is worrying me is that it's not clear how
    much user-visible behavioral change will result, and therefore I'm not
    entirely comfortable with the idea of back-patching such a change into
    9.0 --- and it wouldn't work at all in 8.4, where there simply isn't a
    good opportunity to set a typmod for parameters passed to the main
    executor (since the SPI interfaces plpgsql uses don't support that).
    Attached is a proposed patch for HEAD that sets up the Param's typmod
    sanely.  I've verified that this fixes the reported problem and does not
    result in any changes in the regression tests, which makes me a bit more
    optimistic about it ... but I'm still not convinced it'd be a good idea
    to back-patch into 9.0.
    Back-patching doesn't seem very safe to me, either; though I'm not
    entirely certain what to do instead. Relaxing the check, as you
    proposed upthread, might be the way to go.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedMay 12, '11 at 9:06p
activeMay 14, '11 at 2:17a
posts3
users2
websitepostgresql.org...
irc#postgresql

2 users in discussion

Tom Lane: 2 posts Robert Haas: 1 post

People

Translate

site design / logo © 2022 Grokbase