This is a discussion on suspicious pointer/integer coersion within the pgsql Hackers forums, part of the PostgreSQL category; --> I have just noticed this code in plperl.c: hv_store(plperl_proc_hash, internal_proname, proname_len, newSViv((IV) prodesc), 0); basically, here prodesc is a ...
| |||||||
| FAQ | Members List | Calendar | Search | Today's Posts | Mark Forums Read |
| ||||
| I have just noticed this code in plperl.c: hv_store(plperl_proc_hash, internal_proname, proname_len, newSViv((IV) prodesc), 0); basically, here prodesc is a pointer to a struct, and we are storing it as an integer in a perl hash for easy lookup by stringified oid. How safe is this conversion on 64 bit platforms? 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 |
| |||
| Andrew Dunstan <andrew@dunslane.net> writes: > I have just noticed this code in plperl.c: > hv_store(plperl_proc_hash, internal_proname, proname_len, > newSViv((IV) prodesc), 0); > basically, here prodesc is a pointer to a struct, and we are storing it > as an integer in a perl hash for easy lookup by stringified oid. > How safe is this conversion on 64 bit platforms? Not at all. I'd vote for converting the whole thing to a dynahash hashtable ... regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 4: Have you searched our list archives? http://archives.postgresql.org |
| |||
| Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>I have just noticed this code in plperl.c: >> >> > > > >> hv_store(plperl_proc_hash, internal_proname, proname_len, >> newSViv((IV) prodesc), 0); >> >> > > > >>basically, here prodesc is a pointer to a struct, and we are storing it >>as an integer in a perl hash for easy lookup by stringified oid. >> >> > > > >>How safe is this conversion on 64 bit platforms? >> >> > >Not at all. I'd vote for converting the whole thing to a dynahash >hashtable ... > > > > Works for me. There are some other things about the procdesc stuff I'm trying to sort out (especially if we should be storing per-call info inside it). Cleaning this up is definitely a TODO. cheers andrew ---------------------------(end of broadcast)--------------------------- TIP 2: Don't 'kill -9' the postmaster |
| |||
| Andrew Dunstan <andrew@dunslane.net> writes: > Works for me. There are some other things about the procdesc stuff I'm > trying to sort out (especially if we should be storing per-call info > inside it). Hmm, probably not ... check to see if a recursive plperl function behaves sanely. (This might not have been much of an issue before we had SPI support in plperl, since there was no way to recurse; but it is an issue now.) regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 4: Have you searched our list archives? http://archives.postgresql.org |
| |||
| Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>Works for me. There are some other things about the procdesc stuff I'm >>trying to sort out (especially if we should be storing per-call info >>inside it). >> >> > >Hmm, probably not ... check to see if a recursive plperl function >behaves sanely. (This might not have been much of an issue before >we had SPI support in plperl, since there was no way to recurse; >but it is an issue now.) > > Behaviour is not good (see below for proof). ISTM we'll need some sort of implicit of explicit stack of per-call data. The trick will be getting it to behave right under error recovery. cheers andrew [andrew inst]$ bin/psql -e -f recurse.sql create or replace function recurse(i int) returns setof text language plperl as $$ my $i = shift; elog(NOTICE,"i = $i"); foreach my $x (1..$i) { return_next "hello $x"; } if ($i > 2) { my $z = $i-1; my $cursor = spi_query("select * from recurse($z)"); while (defined(my $row = spi_fetchrow($cursor))) { return_next "recurse $i: $row"; } } return undef; $$; CREATE FUNCTION select * from recurse(2); psql:recurse.sql:24: NOTICE: i = 2 recurse --------- hello 1 hello 2 (2 rows) select * from recurse(3); psql:recurse.sql:25: NOTICE: i = 3 psql:recurse.sql:25: NOTICE: i = 2 psql:recurse.sql:25: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. psql:recurse.sql:25: connection to server was lost [andrew inst]$ ---------------------------(end of broadcast)--------------------------- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq |
| |||
| Andrew Dunstan wrote: > Tom Lane wrote: > >> Andrew Dunstan <andrew@dunslane.net> writes: >> >>> Works for me. There are some other things about the procdesc stuff >>> I'm trying to sort out (especially if we should be storing per-call >>> info inside it). >>> >> >> >> Hmm, probably not ... check to see if a recursive plperl function >> behaves sanely. (This might not have been much of an issue before >> we had SPI support in plperl, since there was no way to recurse; >> but it is an issue now.) > > > Behaviour is not good (see below for proof). > > ISTM we'll need some sort of implicit of explicit stack of per-call > data. The trick will be getting it to behave right under error recovery. Looking further ... we already do this implicitly for prodesc in the call handler - we would just need to do the same thing for per-call structures and divorce them from prodesc, which can be repeated on the implicit stack. I'll work on that - changes should be quite small. cheers andrew ---------------------------(end of broadcast)--------------------------- TIP 6: explain analyze is your friend |
| |||
| > Looking further ... we already do this implicitly for prodesc in the > call handler - we would just need to do the same thing for per-call > structures and divorce them from prodesc, which can be repeated on the > implicit stack. > > I'll work on that - changes should be quite small. Sounds like recursive calls are somethign that should be tested for PLs in the regressions... Chris ---------------------------(end of broadcast)--------------------------- TIP 2: Don't 'kill -9' the postmaster |
| |||
| Andrew Dunstan wrote: > > > > > Looking further ... we already do this implicitly for prodesc in the > call handler - we would just need to do the same thing for per-call > structures and divorce them from prodesc, which can be repeated on the > implicit stack. > > I'll work on that - changes should be quite small. > Attached is a patch that fixes both a recently introduced problem with recursion and a problem with array returns that became evident as a result of not throwing away non-fatal warnings (thanks to David Fetter for noticing this). Regression test updates to include both cases are included in the patch. I will start looking at putting the procedure descriptors in a dynahash. cheers andrew ---------------------------(end of broadcast)--------------------------- TIP 4: Have you searched our list archives? http://archives.postgresql.org |
| ||||
| Andrew Dunstan wrote: > > > Andrew Dunstan wrote: > >> >> >> >> >> Looking further ... we already do this implicitly for prodesc in the >> call handler - we would just need to do the same thing for per-call >> structures and divorce them from prodesc, which can be repeated on >> the implicit stack. >> >> I'll work on that - changes should be quite small. >> > > Attached is a patch that fixes (I hope) both a recently introduced > problem with recursion and a problem with array returns that became > evident as a result of not throwing away non-fatal warnings (thanks to > David Fetter for noticing this). Regression test updates to include > both cases are included in the patch. > > I will start looking at putting the procedure descriptors in a dynahash. > > and here's the patch this time. cheers andrew ---------------------------(end of broadcast)--------------------------- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq |