Re: [PATCHES] plperl - put schema-name in $_TD
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Adam Sjøgren wrote: Enclosed is a tiny patch for plperl that puts the schema-name of the current table in $_TD, so triggers can access tables using schemaname.tablename, for instance like so: This seems like a good idea, but we should probably make analogous changes for plpgsql, pltcl and plpython. Having different trigger data available in some of these doesn't seem like a good idea. Yeah. I'm also a little disturbed by using nspname which is an entirely internal name; plus it's a bit unclear *which* schema it's supposed to be. (One might think it's the schema the trigger function is in, for instance.) Somebody established a bad precedent by using relname for the table name. Maybe we should use field names like table_name and table_schema. relname could be kept around for awhile but deprecated as a duplicate of table_name. Or if that seems too messy, keep relname but use relschema as the new field. regards, tom lane Here are the various bits of trigger data our languages get: plpgsql (function variables) : NEW OLD TG_NAME TG_WHEN TG_LEVEL TG_OP TG_RELID TG_RELNAME TH_NARGS TG_ARGV[] plperl (keys in %$_TD): new old name event when level relid relname argc args plpython (keys of TD): new old name event when level relid args pltcl: (function variables) $TG_name $TG_relid $TG_relatts $TG_when $TG_level $TG_op $NEW $OLD $args plpython and pltcl don't have relname, while only pltcl has relatts. Is relatts useful? should we provide it everywhere? I propose to add relname to plpython and pltcl, and relschema to all (mutatis mutandis w.r.t. names). cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plperl - put schema-name in $_TD
Andrew Dunstan [EMAIL PROTECTED] writes: plpython and pltcl don't have relname, while only pltcl has relatts. Is relatts useful? should we provide it everywhere? Hm. It is not particularly useful in plpgsql at the moment, because of the lack of any way to reference columns dynamically. So that's probably why it's not there in plpgsql, and then the other languages copied that decision even though they can do dynamic references. You'd have to work out appropriate datastructure idioms for the other languages, which might not be obvious at first glance. It doesn't seem very high priority to me, because no one's yet asked for it ... I propose to add relname to plpython and pltcl, and relschema to all (mutatis mutandis w.r.t. names). Works for me. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plperl - put schema-name in $_TD
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 plpython and pltcl don't have relname, while only pltcl has relatts. Is relatts useful? should we provide it everywhere? Might as well - does no harm to add it in. I propose to add relname to plpython and pltcl, and relschema to all (mutatis mutandis w.r.t. names). +1 for table_schema and table_name, especially if in the future we provide things like trigger_schema. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200605251203 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFEddWlvJuQZxSWSsgRApwyAKCyzFMxO4mnW+1CFVugi4K09rLLdwCcDAgx A5sn8irFjBwTa4kNLEITjec= =YcSr -END PGP SIGNATURE- ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plperl - put schema-name in $_TD
On Wed, 24 May 2006 10:12:17 -0400, Andrew wrote:
Patches should be made against the HEAD branch in CVS, not against a
distro source.
Ok; I'll do that. (The patch did apply cleanly to CVS, though, but
anyway).
On Wed, 24 May 2006 15:41:07 -0400, Tom wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
This seems like a good idea, but we should probably make analogous
changes for plpgsql, pltcl and plpython. Having different trigger data
available in some of these doesn't seem like a good idea.
Yeah. I'm also a little disturbed by using nspname which is an
entirely internal name; plus it's a bit unclear *which* schema it's
supposed to be. (One might think it's the schema the trigger function
is in, for instance.) Somebody established a bad precedent by using
relname for the table name.
I wasn't sure what to call it, so I modelled my change after relname ~
SPI_getrelname and arrived at the questionable nspname ~ SPI_getnspname.
Maybe we should use field names like table_name and table_schema.
relname could be kept around for awhile but deprecated as a duplicate
of table_name.
On Thu, 25 May 2006 16:06:12 -, Greg wrote:
+1 for table_schema and table_name, especially if in the future we provide
things like trigger_schema.
I've attached a new patch, against CVS, that adds table_name and
table_schema instead, and updates the doc accordingly.
I haven't looked at the other languages as I do not use them; let me
know if I should take a stab at providing patches for them as well.
Thanks for your comments.
Best regards,
Adam
--
Our hero regains consciousness at the feet of a Adam Sjøgren
sarcastic alien...[EMAIL PROTECTED]
? patch
Index: doc/src/sgml/plperl.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/plperl.sgml,v
retrieving revision 2.52
diff -c -r2.52 plperl.sgml
*** doc/src/sgml/plperl.sgml 10 Mar 2006 19:10:48 - 2.52
--- doc/src/sgml/plperl.sgml 25 May 2006 18:49:34 -
***
*** 728,734
/varlistentry
varlistentry
! termliteral$_TD-gt;{relname}/literal/term
listitem
para
Name of the table on which the trigger fired
--- 728,734
/varlistentry
varlistentry
! termliteral$_TD-gt;{table_name}/literal/term
listitem
para
Name of the table on which the trigger fired
***
*** 737,742
--- 737,760
/varlistentry
varlistentry
+ termliteral$_TD-gt;{relname}/literal/term
+ listitem
+ para
+Name of the table on which the trigger fired. This has been deprecated. Please use $_TD-gt;{table_name} instead.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
+ termliteral$_TD-gt;{table_schema}/literal/term
+ listitem
+ para
+Name of the schema in which the table on which the trigger fired, is
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termliteral$_TD-gt;{argc}/literal/term
listitem
para
Index: src/pl/plperl/plperl.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.108
diff -c -r1.108 plperl.c
*** src/pl/plperl/plperl.c 4 Apr 2006 19:35:37 - 1.108
--- src/pl/plperl/plperl.c 25 May 2006 18:49:37 -
***
*** 525,530
--- 525,536
hv_store(hv, relname, 7,
newSVpv(SPI_getrelname(tdata-tg_relation), 0), 0);
+ hv_store(hv, table_name, 10,
+ newSVpv(SPI_getrelname(tdata-tg_relation), 0), 0);
+
+ hv_store(hv, table_schema, 12,
+ newSVpv(SPI_getnspname(tdata-tg_relation), 0), 0);
+
if (TRIGGER_FIRED_BEFORE(tdata-tg_event))
when = BEFORE;
else if (TRIGGER_FIRED_AFTER(tdata-tg_event))
---(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: [PATCHES] plperl - put schema-name in $_TD
Adam Sjøgren said: I haven't looked at the other languages as I do not use them; let me know if I should take a stab at providing patches for them as well. I will take it from here. Thanks. andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] plperl - put schema-name in $_TD
Adam Sjøgren wrote:
Hi.
Enclosed is a tiny patch for plperl that puts the schema-name of the
current table in $_TD, so triggers can access tables using
schemaname.tablename, for instance like so:
my $query='INSERT INTO ' . $_TD-{nspname} . '.' . $_TD-{relname} . '_archive
(' . $fieldnames . ') VALUES(' . $values . ');';
(This way my triggers can work without setting search_path).
The patch was made against PostgreSQL 8.1.3 in Ubuntu dapper.
This is my first patch for PostgreSQL, so any advice, comments,
pointers, etc. are very welcome.
Patches should be made against the HEAD branch in CVS, not against a
distro source.
This seems like a good idea, but we should probably make analogous
changes for plpgsql, pltcl and plpython. Having different trigger data
available in some of these doesn't seem like a good idea.
cheers
andrew
Best regards,
Adam
*** src/pl/plperl/plperl.c.orig 2006-05-23 16:57:25.0 +0200
--- src/pl/plperl/plperl.c 2006-05-23 16:57:45.0 +0200
***
*** 550,555
--- 550,558
hv_store(hv, relname, 7,
newSVpv(SPI_getrelname(tdata-tg_relation), 0), 0);
+ hv_store(hv, nspname, 7,
+ newSVpv(SPI_getnspname(tdata-tg_relation), 0), 0);
+
if (TRIGGER_FIRED_BEFORE(tdata-tg_event))
when = BEFORE;
else if (TRIGGER_FIRED_AFTER(tdata-tg_event))
*** doc/src/sgml/plperl.sgml.orig 2006-05-24 10:06:02.0 +0200
--- doc/src/sgml/plperl.sgml2006-05-24 10:05:49.0 +0200
***
*** 675,680
--- 675,689
/varlistentry
varlistentry
+ termliteral$_TD-gt;{nspname}/literal/term
+ listitem
+ para
+Name of the schema in which the table on which the trigger fired, is
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termliteral$_TD-gt;{argc}/literal/term
listitem
para
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
---(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: [PATCHES] plperl - put schema-name in $_TD
Andrew Dunstan [EMAIL PROTECTED] writes: Adam Sjøgren wrote: Enclosed is a tiny patch for plperl that puts the schema-name of the current table in $_TD, so triggers can access tables using schemaname.tablename, for instance like so: This seems like a good idea, but we should probably make analogous changes for plpgsql, pltcl and plpython. Having different trigger data available in some of these doesn't seem like a good idea. Yeah. I'm also a little disturbed by using nspname which is an entirely internal name; plus it's a bit unclear *which* schema it's supposed to be. (One might think it's the schema the trigger function is in, for instance.) Somebody established a bad precedent by using relname for the table name. Maybe we should use field names like table_name and table_schema. relname could be kept around for awhile but deprecated as a duplicate of table_name. Or if that seems too messy, keep relname but use relschema as the new field. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
