Thread: plperl - put schema-name in $_TD
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.
Best regards,
Adam
*** src/pl/plperl/plperl.c.orig 2006-05-23 16:57:25.000000000 +0200
--- src/pl/plperl/plperl.c 2006-05-23 16:57:45.000000000 +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.000000000 +0200
--- doc/src/sgml/plperl.sgml 2006-05-24 10:05:49.000000000 +0200
***************
*** 675,680 ****
--- 675,689 ----
</varlistentry>
<varlistentry>
+ <term><literal>$_TD->{nspname}</literal></term>
+ <listitem>
+ <para>
+ Name of the schema in which the table on which the trigger fired, is
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>$_TD->{argc}</literal></term>
<listitem>
<para>
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.000000000 +0200
> --- src/pl/plperl/plperl.c 2006-05-23 16:57:45.000000000 +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.000000000 +0200
> --- doc/src/sgml/plperl.sgml 2006-05-24 10:05:49.000000000 +0200
> ***************
> *** 675,680 ****
> --- 675,689 ----
> </varlistentry>
>
> <varlistentry>
> + <term><literal>$_TD->{nspname}</literal></term>
> + <listitem>
> + <para>
> + Name of the schema in which the table on which the trigger fired, is
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term><literal>$_TD->{argc}</literal></term>
> <listitem>
> <para>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
>
>
Andrew Dunstan <andrew@dunslane.net> 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
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> 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
Andrew Dunstan <andrew@dunslane.net> 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
-----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 greg@turnstep.com PGP Key: 0x14964AC8 200605251203 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iD8DBQFEddWlvJuQZxSWSsgRApwyAKCyzFMxO4mnW+1CFVugi4K09rLLdwCcDAgx A5sn8irFjBwTa4kNLEITjec= =YcSr -----END PGP SIGNATURE-----
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 <andrew@dunslane.net> 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 -0000, 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..." asjo@koldfront.dk
? 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 -0000 2.52
--- doc/src/sgml/plperl.sgml 25 May 2006 18:49:34 -0000
***************
*** 728,734 ****
</varlistentry>
<varlistentry>
! <term><literal>$_TD->{relname}</literal></term>
<listitem>
<para>
Name of the table on which the trigger fired
--- 728,734 ----
</varlistentry>
<varlistentry>
! <term><literal>$_TD->{table_name}</literal></term>
<listitem>
<para>
Name of the table on which the trigger fired
***************
*** 737,742 ****
--- 737,760 ----
</varlistentry>
<varlistentry>
+ <term><literal>$_TD->{relname}</literal></term>
+ <listitem>
+ <para>
+ Name of the table on which the trigger fired. This has been deprecated. Please use $_TD->{table_name}
instead.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>$_TD->{table_schema}</literal></term>
+ <listitem>
+ <para>
+ Name of the schema in which the table on which the trigger fired, is
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>$_TD->{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 -0000 1.108
--- src/pl/plperl/plperl.c 25 May 2006 18:49:37 -0000
***************
*** 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))
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