Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
What about plpgsql variables, ie: DECLARE v varchar(42); BEGIN ... On Jan 26, 2007, at 9:48 PM, Andrew Dunstan wrote: Bruce Momjian wrote: OK, what is the TODO wording?cheers Something like: Enforce typmod for function inputs, function results and parameters for spi_prepare'd statements called from PLs. cheers andrew -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
Bruce Momjian wrote: OK, what is the TODO wording?cheers Something like: Enforce typmod for function inputs, function results and parameters for spi_prepare'd statements called from PLs. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
Jim Nasby wrote: On Jan 26, 2007, at 9:31 AM, Tom Lane wrote: If you wanted to be a bit more ambitious maybe you could change the fact that this code is throwing away typmod, which means that declarations like "varchar(32)" would fail to work as expected. Perhaps it should be fixed to save the typmods alongside the typioparams and then pass them to InputFunctionCall instead of passing -1. On the other hand, we don't currently enforce typmod for any function input or result arguments, so maybe it's consistent that spi_prepare arguments ignore typmods too. Thoughts? I'd like to see us move towards supporting that; both for function parameters/results as well as inside functions. It'd be nice if both cases got fixed at once, but IMHO fixing only one now would be better than fixing none. I'm not going to do either in fixing this bug - I think they should be fixed but are a separate issue. These probably belong on the TODO list. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
On Jan 26, 2007, at 9:31 AM, Tom Lane wrote: If you wanted to be a bit more ambitious maybe you could change the fact that this code is throwing away typmod, which means that declarations like "varchar(32)" would fail to work as expected. Perhaps it should be fixed to save the typmods alongside the typioparams and then pass them to InputFunctionCall instead of passing -1. On the other hand, we don't currently enforce typmod for any function input or result arguments, so maybe it's consistent that spi_prepare arguments ignore typmods too. Thoughts? I'd like to see us move towards supporting that; both for function parameters/results as well as inside functions. It'd be nice if both cases got fixed at once, but IMHO fixing only one now would be better than fixing none. -- Jim Nasby[EMAIL PROTECTED] EnterpriseDB http://enterprisedb.com 512.569.9461 (cell) ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
Andrew Dunstan <[EMAIL PROTECTED]> writes: > see attached patch. If this is OK I will apply it and also fix pltcl > and plpython similarly, mutatis mutandis. Looks alright as far as it goes, but I'd suggest making one additional cleanup while you're in there: get rid of the direct syscache access altogether, instead using getTypeInputInfo(). The loop body should just consist of three function calls: parseTypeString, getTypeInputInfo, perm_fmgr_info. If you wanted to be a bit more ambitious maybe you could change the fact that this code is throwing away typmod, which means that declarations like "varchar(32)" would fail to work as expected. Perhaps it should be fixed to save the typmods alongside the typioparams and then pass them to InputFunctionCall instead of passing -1. On the other hand, we don't currently enforce typmod for any function input or result arguments, so maybe it's consistent that spi_prepare arguments ignore typmods too. Thoughts? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
I wrote: Tom Lane wrote: I think parseTypeString() may be the thing to use. It's what plpgsql uses... OK, I'll see what I can do. see attached patch. If this is OK I will apply it and also fix pltcl and plpython similarly, mutatis mutandis. cheers andrew Index: src/pl/plperl/plperl.c === RCS file: /cvsroot/pgsql/src/pl/plperl/plperl.c,v retrieving revision 1.123 diff -c -r1.123 plperl.c *** src/pl/plperl/plperl.c 21 Nov 2006 16:59:02 - 1.123 --- src/pl/plperl/plperl.c 26 Jan 2007 15:13:05 - *** *** 2128,2145 PG_TRY(); { / ! * Lookup the argument types by name in the system cache ! * and remember the required information for input conversion / for (i = 0; i < argc; i++) { ! List *names; HeapTuple typeTup; ! /* Parse possibly-qualified type name and look it up in pg_type */ ! names = stringToQualifiedNameList(SvPV(argv[i], PL_na), ! "plperl_spi_prepare"); ! typeTup = typenameType(NULL, makeTypeNameFromNameList(names)); qdesc->argtypes[i] = HeapTupleGetOid(typeTup); perm_fmgr_info(((Form_pg_type) GETSTRUCT(typeTup))->typinput, &(qdesc->arginfuncs[i])); --- 2128,2152 PG_TRY(); { / ! * Resolve argument type names and then look them up by oid ! * in the system cache, and remember the required information ! * for input conversion. / for (i = 0; i < argc; i++) { ! Oid typeId; ! int32 typmod; HeapTuple typeTup; ! parseTypeString(SvPV(argv[i], PL_na), &typeId, &typmod); ! ! typeTup = SearchSysCache(TYPEOID, ! ObjectIdGetDatum(typeId), ! 0,0,0); ! if (!HeapTupleIsValid(typeTup)) ! elog(ERROR, "cache lookup failed for type %u", typeId); ! ! qdesc->argtypes[i] = HeapTupleGetOid(typeTup); perm_fmgr_info(((Form_pg_type) GETSTRUCT(typeTup))->typinput, &(qdesc->arginfuncs[i])); Index: src/pl/plperl/expected/plperl.out === RCS file: /cvsroot/pgsql/src/pl/plperl/expected/plperl.out,v retrieving revision 1.9 diff -c -r1.9 plperl.out *** src/pl/plperl/expected/plperl.out 13 Aug 2006 17:31:10 - 1.9 --- src/pl/plperl/expected/plperl.out 26 Jan 2007 15:13:05 - *** *** 438,444 -- Test spi_prepare/spi_exec_prepared/spi_freeplan -- CREATE OR REPLACE FUNCTION perl_spi_prepared(INTEGER) RETURNS INTEGER AS $$ !my $x = spi_prepare('select $1 AS a', 'INT4'); my $q = spi_exec_prepared( $x, $_[0] + 1); spi_freeplan($x); return $q->{rows}->[0]->{a}; --- 438,444 -- Test spi_prepare/spi_exec_prepared/spi_freeplan -- CREATE OR REPLACE FUNCTION perl_spi_prepared(INTEGER) RETURNS INTEGER AS $$ !my $x = spi_prepare('select $1 AS a', 'INTEGER'); my $q = spi_exec_prepared( $x, $_[0] + 1); spi_freeplan($x); return $q->{rows}->[0]->{a}; *** *** 468,470 --- 468,504 4 (2 rows) + -- + -- Test prepare with a type with spaces + -- + CREATE OR REPLACE FUNCTION perl_spi_prepared_double(double precision) RETURNS double precision AS $$ + my $x = spi_prepare('SELECT 10.0 * $1 AS a', 'DOUBLE PRECISION'); + my $q = spi_query_prepared($x,$_[0]); + my $result; + while (defined (my $y = spi_fetchrow($q))) { + $result = $y->{a}; + } + spi_freeplan($x); + return $result; + $$ LANGUAGE plperl; + SELECT perl_spi_prepared_double(4.35) as "double precision"; + double precision + -- + 43.5 + (1 row) + + -- + -- Test with a bad type + -- + CREATE OR REPLACE FUNCTION perl_spi_prepared_bad(double precision) RETURNS double precision AS $$ + my $x = spi_prepare('SELECT 10.0 * $1 AS a', 'does_not_exist'); + my $q = spi_query_prepared($x,$_[0]); + my $result; + while (defined (my $y = spi_fetchrow($q))) { + $result = $y->{a}; + } + spi_freeplan($x); + return $result; + $$ LANGUAGE plperl; + SELECT perl_spi_prepared_bad(4.35) as "double precision"; + ERROR: error from Perl function: type "does_not_exist" does not exist at line 2. Index: src/pl/plperl/sql/plperl.sql === RCS file: /cvsroot/pgsql/src/pl/plperl/sql/plperl.sql,v retrieving revision 1.11 diff -c -r1.11 plperl.sql *** src/pl/plperl/sql/plperl.sql 13 Aug 2006 17:31:10 - 1.11 --- src/pl/plperl/sql/plperl.sql 26 Jan 2007 15:13:05 - *** *** 316,322 -- Test spi_prepare/spi_exec_prepared/spi_freeplan -- CREATE OR REPLACE FUNCTION perl_spi_prepared(INTEGER) RETURNS INTEGER AS $$ !my $x = spi_prepare('se
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: I dug into it a bit and found that pltcl and plpython appear to use almost identical code, but only pltcl has this limitation documented. I'm inclined to say we should document this for plperl and plpython for stable releases and remove the limitation for all three for 8.3. I see that SQL level prepare calls regprocin() to resolve type names, so maybe we should that for the PLs when calling SPI_prepare as well. I think parseTypeString() may be the thing to use. It's what plpgsql uses... OK, I'll see what I can do. cheers andrew. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: I see that SQL level prepare calls regprocin() to resolve type names, so maybe we should that for the PLs when calling SPI_prepare as well. Of course, that should be regtypein() [ squint... ] build_regtype_array seems a rather stupid bit of code. How many hundreds of cycles is it expending to convert an OID to an OID? Yeah, you're right. I should have squinted too ;-) Well, I am open to suggestions on what to do about this. I really think we should support use of standard type aliases in the PLs. cheers andrew ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases such as 'integer'
Andrew Dunstan <[EMAIL PROTECTED]> writes: > I dug into it a bit and found that pltcl and plpython appear to use > almost identical code, but only pltcl has this limitation documented. > I'm inclined to say we should document this for plperl and plpython for > stable releases and remove the limitation for all three for 8.3. I see > that SQL level prepare calls regprocin() to resolve type names, so maybe > we should that for the PLs when calling SPI_prepare as well. I think parseTypeString() may be the thing to use. It's what plpgsql uses... regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
Andrew Dunstan <[EMAIL PROTECTED]> writes: >> I see that SQL level prepare calls regprocin() to resolve type names, >> so maybe we should that for the PLs when calling SPI_prepare as well. > Of course, that should be regtypein() [ squint... ] build_regtype_array seems a rather stupid bit of code. How many hundreds of cycles is it expending to convert an OID to an OID? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases
I wrote: I see that SQL level prepare calls regprocin() to resolve type names, so maybe we should that for the PLs when calling SPI_prepare as well. Of course, that should be regtypein() cheers andrew ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] BUG #2917: spi_prepare doesn't accept typename aliases such as 'integer'
The author of this bug was good enough to send me a copy, since I don't normally read the -bugs list. It can now be found at http://archives.postgresql.org/pgsql-bugs/2007-01/msg00111.php . I dug into it a bit and found that pltcl and plpython appear to use almost identical code, but only pltcl has this limitation documented. I'm inclined to say we should document this for plperl and plpython for stable releases and remove the limitation for all three for 8.3. I see that SQL level prepare calls regprocin() to resolve type names, so maybe we should that for the PLs when calling SPI_prepare as well. Alternatively, should we adjust things lower down (e.g. in typenameType() or LookupTypeName() ) ? Comments? cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster