How to extract a value from a record using attnum or attname?

Started by Kevin Grittneralmost 15 years ago15 messages
#1Kevin Grittner
Kevin.Grittner@wicourts.gov

PL/pgSQL seems tantalizingly close to being useful for developing a
generalized trigger function for notifying the client of changes. I
don't know whether I'm missing something or whether we're missing a
potentially useful feature here. Does anyone see how to fill in
where the commented question is, or do I need to write this function
in C?

Alternatively, I guess, I could write a C-based
quote_literal(record, int2) and/or quote_literal(record, name)
function to use there.

create or replace function tcn_notify() returns trigger
language plpgsql as $tcn_notify$
declare
keycols int2vector;
keycolname text;
channel text;
payload text;
begin
select indkey from pg_catalog.pg_index
where indrelid = tg_relid and indisprimary
into keycols;
if not found then
raise exception 'no primary key found for table %.%',
quote_ident(tg_table_schema), quote_ident(tg_table_name);
end if;
channel := 'tcn' || pg_backend_pid()::text;
payload := quote_ident(tg_table_name) || ','
|| substring(tg_op, 1, 1);
for i in array_lower(keycols, 1)..array_upper(keycols, 1) loop
select quote_ident(attname) from pg_catalog.pg_attribute
where attrelid = tg_relid and attnum = keycols[i]::oid
into keycolname;
payload := payload || ',' || keycolname || '=';

-- How do I append the quote_literal(value) ?????

end loop;
perform pg_notify(channel, payload);
return null; -- ignored because this is an AFTER trigger
end;
$tcn_notify$;

It would surprise me if nobody else has wanted to do something like
this. The only reason we hadn't hit it yet is that we'd been
striving for portable code and had been doing such things in a Java
tier outside the database.

-Kevin

#2Thomas Kellerer
spam_eater@gmx.net
In reply to: Kevin Grittner (#1)
Re: How to extract a value from a record using attnum or attname?

Kevin Grittner wrote on 04.02.2011 23:27:

PL/pgSQL seems tantalizingly close to being useful for developing a
generalized trigger function for notifying the client of changes. I
don't know whether I'm missing something or whether we're missing a
potentially useful feature here. Does anyone see how to fill in
where the commented question is, or do I need to write this function
in C?

Alternatively, I guess, I could write a C-based
quote_literal(record, int2) and/or quote_literal(record, name)
function to use there.

create or replace function tcn_notify() returns trigger
language plpgsql as $tcn_notify$
declare
keycols int2vector;
keycolname text;
channel text;
payload text;
begin
select indkey from pg_catalog.pg_index
where indrelid = tg_relid and indisprimary
into keycols;
if not found then
raise exception 'no primary key found for table %.%',
quote_ident(tg_table_schema), quote_ident(tg_table_name);
end if;
channel := 'tcn' || pg_backend_pid()::text;
payload := quote_ident(tg_table_name) || ','
|| substring(tg_op, 1, 1);
for i in array_lower(keycols, 1)..array_upper(keycols, 1) loop
select quote_ident(attname) from pg_catalog.pg_attribute
where attrelid = tg_relid and attnum = keycols[i]::oid
into keycolname;
payload := payload || ',' || keycolname || '=';

-- How do I append the quote_literal(value) ?????

end loop;
perform pg_notify(channel, payload);
return null; -- ignored because this is an AFTER trigger
end;
$tcn_notify$;

It would surprise me if nobody else has wanted to do something like
this. The only reason we hadn't hit it yet is that we'd been
striving for portable code and had been doing such things in a Java
tier outside the database.

If you don't really need the key = value pairs, you can simply use:

payload := payload || 'values: ' || ROW(old.*);

this will append everything in one operation, but not in the col=value format

Regards
Thomas

#3Dimitri Fontaine
dimitri@2ndQuadrant.fr
In reply to: Kevin Grittner (#1)
Re: How to extract a value from a record using attnum or attname?

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

PL/pgSQL seems tantalizingly close to being useful for developing a
generalized trigger function for notifying the client of changes. I
don't know whether I'm missing something or whether we're missing a
potentially useful feature here. Does anyone see how to fill in
where the commented question is, or do I need to write this function
in C?

See those:

http://tapoueh.org/articles/blog/_Dynamic_Triggers_in_PLpgSQL.html
http://www.pgsql.cz/index.php/PL_toolbox_%28en%29#Record.27s_functions

for i in array_lower(keycols, 1)..array_upper(keycols, 1) loop
select quote_ident(attname) from pg_catalog.pg_attribute
where attrelid = tg_relid and attnum = keycols[i]::oid

Beware of attisdropped, which I've not fixed in the published URL
before (the tapoueh.org one).

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

#4Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Dimitri Fontaine (#3)
1 attachment(s)
Re: How to extract a value from a record using attnum or attname?

[moving to -hackers with BC to -general]

Dimitri Fontaine <dimitri@2ndQuadrant.fr> wrote:

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:

PL/pgSQL seems tantalizingly close to being useful for developing
a generalized trigger function for notifying the client of
changes. I don't know whether I'm missing something or whether
we're missing a potentially useful feature here. Does anyone see
how to fill in where the commented question is, or do I need to
write this function in C?

See those:

http://tapoueh.org/articles/blog/_Dynamic_Triggers_in_PLpgSQL.html

http://www.pgsql.cz/index.php/PL_toolbox_%28en%29#Record.27s_functions

for i in array_lower(keycols, 1)..array_upper(keycols, 1) loop
select quote_ident(attname) from pg_catalog.pg_attribute
where attrelid = tg_relid and attnum = keycols[i]::oid

Beware of attisdropped, which I've not fixed in the published URL
before (the tapoueh.org one).

Thanks.

In the absence of an earlier response, though, I went ahead and
wrote the attached, which has passed some initial programmer testing
and is scheduled to start business analyst testing tomorrow with the
application software for production deployment in a couple months.
We probably won't go back to PL/pgSQL for this now.

I'm assuming that while I have an AccessShareLock on the index
relation for the primary key, any attributes it tells me are used by
that relation will not have the attisdropped flag set?

What this trigger function does is to issue a NOTIFY to the channel
specified as a parameter to the function in CREATE TRIGGER (with
'tcn' as the default), and a payload consisting of the table name, a
code for the operation (Insert, Update, or Delete), and the primary
key values. So, an update to a Party record for us might generate
this NOTIFY payload:

"Party",U,"countyNo"='71',"caseNo"='2011CF001234',"partyNo"='1'

This is one of those things which our shop needs, but I was planning
to post it for the first 9.2 CF fest to see if anyone else was
interested. It struck me while typing this post that for general
use the schema would probably need to be in there, but I'll worry
about that later, if anyone else *is* interested. If anyone wants
it I can provide Java code to tear apart the NOTIFY payloads using
the Pattern and Matches classes.

I'll add to the first 9.2 CF referencing this post.

-Kevin

Attachments:

tcn-1.patchtext/plain; name=tcn-1.patchDownload
*** a/src/backend/utils/adt/trigfuncs.c
--- b/src/backend/utils/adt/trigfuncs.c
***************
*** 13,21 ****
   */
  #include "postgres.h"
  
! #include "access/htup.h"
  #include "commands/trigger.h"
  #include "utils/builtins.h"
  
  
  /*
--- 13,25 ----
   */
  #include "postgres.h"
  
! #include "executor/spi.h"
! #include "catalog/indexing.h"
! #include "commands/async.h"
  #include "commands/trigger.h"
  #include "utils/builtins.h"
+ #include "utils/fmgroids.h"
+ #include "utils/tqual.h"
  
  
  /*
***************
*** 93,95 **** suppress_redundant_updates_trigger(PG_FUNCTION_ARGS)
--- 97,261 ----
  
  	return PointerGetDatum(rettuple);
  }
+ 
+ 
+ /*
+  * Copy from s (for source) to r (for result), wrapping with q (quote)
+  * characters and doubling any quote characters found.
+  */
+ static char *
+ strcpy_quoted(char *r, const char *s, const char q)
+ {
+ 	*r++ = q;
+ 	while (*s)
+ 	{
+ 		if (*s == q)
+ 			*r++ = q;
+ 		*r++ = *s;
+ 		s++;
+ 	}
+ 	*r++ = q;
+ 	return r;
+ }
+ 
+ /*
+  * triggered_change_notification
+  *
+  * This trigger function will send a notification of data modification with
+  * primary key values.	The channel will be "tcn" unless the trigger is
+  * created with a parameter, in which case that parameter will be used.
+  */
+ Datum
+ triggered_change_notification(PG_FUNCTION_ARGS)
+ {
+ 	TriggerData *trigdata = (TriggerData *) fcinfo->context;
+ 	Trigger    *trigger;
+ 	int			nargs;
+ 	HeapTuple	trigtuple,
+ 				newtuple;
+ 	HeapTupleHeader trigheader,
+ 				newheader;
+ 	Relation	rel;
+ 	TupleDesc	tupdesc;
+ 	Relation	indexRelation;
+ 	ScanKeyData skey;
+ 	SysScanDesc scan;
+ 	HeapTuple	indexTuple;
+ 	char	   *channel;
+ 	char		operation;
+ 	char		payload[200];
+ 	char	   *p;
+ 	bool		foundPK;
+ 
+ 	/* make sure it's called as a trigger */
+ 	if (!CALLED_AS_TRIGGER(fcinfo))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 		errmsg("triggered_change_notification: must be called as trigger")));
+ 
+ 	/* and that it's called after the change */
+ 	if (!TRIGGER_FIRED_AFTER(trigdata->tg_event))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("triggered_change_notification: must be called after the change")));
+ 
+ 	/* and that it's called for each row */
+ 	if (!TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("suppress_redundant_updates_trigger: must be called for each row")));
+ 
+ 	if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
+ 		operation = 'I';
+ 	else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
+ 		operation = 'U';
+ 	else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
+ 		operation = 'D';
+ 	else
+ 	{
+ 		elog(ERROR, "suppress_redundant_updates_trigger: trigger fired by unrecognized operation");
+ 		operation = 'X';		/* silence compiler warning */
+ 	}
+ 
+ 	trigger = trigdata->tg_trigger;
+ 	nargs = trigger->tgnargs;
+ 	if (nargs > 1)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("suppress_redundant_updates_trigger: must not be called with more than one parameter")));
+ 
+ 	if (nargs == 0)
+ 		channel = "tcn";
+ 	else
+ 		channel = trigger->tgargs[0];
+ 
+ 	/* get tuple data, set default result */
+ 	trigtuple = trigdata->tg_trigtuple;
+ 	newtuple = trigdata->tg_newtuple;
+ 
+ 	trigheader = trigtuple->t_data;
+ 	newheader = newtuple->t_data;
+ 
+ 	rel = trigdata->tg_relation;
+ 	tupdesc = rel->rd_att;
+ 
+ 	foundPK = false;
+ 
+ 	/* Prepare to scan pg_index for entries having indrelid = this rel. */
+ 	indexRelation = heap_open(IndexRelationId, AccessShareLock);
+ 	ScanKeyInit(&skey,
+ 				Anum_pg_index_indrelid,
+ 				BTEqualStrategyNumber, F_OIDEQ,
+ 				ObjectIdGetDatum(RelationGetRelid(rel)));
+ 
+ 	scan = systable_beginscan(indexRelation, IndexIndrelidIndexId, true,
+ 							  SnapshotNow, 1, &skey);
+ 
+ 	while (HeapTupleIsValid(indexTuple = systable_getnext(scan)))
+ 	{
+ 		Form_pg_index index = (Form_pg_index) GETSTRUCT(indexTuple);
+ 
+ 		/* we're only interested if it is the primary key */
+ 		if (index->indisprimary)
+ 		{
+ 			int			numatts = index->indnatts;
+ 
+ 			if (numatts > 0)
+ 			{
+ 				int			i;
+ 
+ 				foundPK = true;
+ 
+ 				p = strcpy_quoted(payload, SPI_getrelname(rel), '"');
+ 				*p++ = ',';
+ 				*p++ = operation;
+ 
+ 				for (i = 0; i < numatts; i++)
+ 				{
+ 					int			colno = index->indkey.values[i];
+ 
+ 					/* TODO: for UPDATE, check that the value wasn't updated? */
+ 
+ 					*p++ = ',';
+ 					p = strcpy_quoted(p, SPI_fname(tupdesc, colno), '"');
+ 					*p++ = '=';
+ 					p = strcpy_quoted(p, SPI_getvalue(trigtuple, tupdesc, colno), '\'');
+ 				}
+ 				*p = '\0';
+ 
+ 				Async_Notify(channel, payload);
+ 			}
+ 			break;
+ 		}
+ 	}
+ 
+ 	systable_endscan(scan);
+ 	heap_close(indexRelation, AccessShareLock);
+ 
+ 	if (!foundPK)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("triggered_change_notification: must be called on a table with a primary key")));
+ 
+ 	return PointerGetDatum(NULL);		/* after trigger; value doesn't matter */
+ }
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 1638,1643 **** DESCR("convert oid to int8");
--- 1638,1645 ----
  
  DATA(insert OID = 1291 (  suppress_redundant_updates_trigger	PGNSP PGUID 12 1 0 0 f f f t f v 0 0 2279 "" _null_ _null_ _null_ _null_ suppress_redundant_updates_trigger _null_ _null_ _null_ ));
  DESCR("trigger to suppress updates when new and old records match");
+ DATA(insert OID = 2650 (  triggered_change_notification	PGNSP PGUID 12 1 0 0 f f f t f v 0 0 2279 "" _null_ _null_ _null_ _null_ triggered_change_notification _null_ _null_ _null_ ));
+ DESCR("trigger function to send change notification with primary key in payload");
  
  DATA(insert OID = 1292 ( tideq			   PGNSP PGUID 12 1 0 0 f f f t f i 2 0 16 "27 27" _null_ _null_ _null_ _null_ tideq _null_ _null_ _null_ ));
  DESCR("equal");
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 959,964 **** extern Datum RI_FKey_setdefault_upd(PG_FUNCTION_ARGS);
--- 959,965 ----
  
  /* trigfuncs.c */
  extern Datum suppress_redundant_updates_trigger(PG_FUNCTION_ARGS);
+ extern Datum triggered_change_notification(PG_FUNCTION_ARGS);
  
  /* encoding support functions */
  extern Datum getdatabaseencoding(PG_FUNCTION_ARGS);
#5Scott Ribe
scott_ribe@elevated-dev.com
In reply to: Dimitri Fontaine (#3)
Re: How to extract a value from a record using attnum or attname?

I don't know if you can quite write the generalized notification function you want in plpgsql or not, but you can certainly write the meta-function that create the function for any table ;-)

--
Scott Ribe
scott_ribe@elevated-dev.com
http://www.elevated-dev.com/
(303) 722-0567 voice

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Kevin Grittner (#4)
Re: Re: [GENERAL] How to extract a value from a record using attnum or attname?

On 02/22/2011 05:32 PM, Kevin Grittner wrote:

[moving to -hackers with BC to -general]

Dimitri Fontaine<dimitri@2ndQuadrant.fr> wrote:

"Kevin Grittner"<Kevin.Grittner@wicourts.gov> writes:

PL/pgSQL seems tantalizingly close to being useful for developing
a generalized trigger function for notifying the client of
changes. I don't know whether I'm missing something or whether
we're missing a potentially useful feature here. Does anyone see
how to fill in where the commented question is, or do I need to
write this function in C?

See those:

http://tapoueh.org/articles/blog/_Dynamic_Triggers_in_PLpgSQL.html

http://www.pgsql.cz/index.php/PL_toolbox_%28en%29#Record.27s_functions

for i in array_lower(keycols, 1)..array_upper(keycols, 1) loop
select quote_ident(attname) from pg_catalog.pg_attribute
where attrelid = tg_relid and attnum = keycols[i]::oid

Beware of attisdropped, which I've not fixed in the published URL
before (the tapoueh.org one).

Thanks.

In the absence of an earlier response, though, I went ahead and
wrote the attached, which has passed some initial programmer testing
and is scheduled to start business analyst testing tomorrow with the
application software for production deployment in a couple months.
We probably won't go back to PL/pgSQL for this now.

I'm assuming that while I have an AccessShareLock on the index
relation for the primary key, any attributes it tells me are used by
that relation will not have the attisdropped flag set?

What this trigger function does is to issue a NOTIFY to the channel
specified as a parameter to the function in CREATE TRIGGER (with
'tcn' as the default), and a payload consisting of the table name, a
code for the operation (Insert, Update, or Delete), and the primary
key values. So, an update to a Party record for us might generate
this NOTIFY payload:

"Party",U,"countyNo"='71',"caseNo"='2011CF001234',"partyNo"='1'

This is one of those things which our shop needs, but I was planning
to post it for the first 9.2 CF fest to see if anyone else was
interested. It struck me while typing this post that for general
use the schema would probably need to be in there, but I'll worry
about that later, if anyone else *is* interested. If anyone wants
it I can provide Java code to tear apart the NOTIFY payloads using
the Pattern and Matches classes.

I'll add to the first 9.2 CF referencing this post.

Have you performance tested it? Scanning pg_index for index columns for
each row strikes me as likely to be unpleasant.

Also, the error messages seem to need a bit of work (no wonder they
seemed familiar to me :) )

cheers

andrew

#7Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Andrew Dunstan (#6)
Re: How to extract a value from a record using attnum or attname?

Andrew Dunstan <andrew@dunslane.net> wrote:

Have you performance tested it? Scanning pg_index for index
columns for each row strikes me as likely to be unpleasant.

I haven't, yet. I had rather assumed that the index info for a
relation would have a high probability of being cached during
execution of an AFTER trigger for that relation, so I think we're
talking RAM access here. It didn't seem sane to try to create an
HTAB for this and worry about invalidation of it, etc. If there's a
faster way to get to the info without going to such extremes, I'd be
happy to hear them. (At least I avoided building and parsing a
query to get at it.)

Also, the error messages seem to need a bit of work (no wonder
they seemed familiar to me :) )

[blush] I was just trying to write code with "fits in with
surrounding code", as recommended. You mean I should change the
function name in the message from the name of the function I copied
*from* to the name of the function I copied *to*? Well, I *guess*
it still fits in.... ;-)

Seriously, thanks for pointing that out. Will fix.

-Kevin

#8Alvaro Herrera
alvherre@commandprompt.com
In reply to: Kevin Grittner (#7)
Re: How to extract a value from a record using attnum or attname?

Excerpts from Kevin Grittner's message of mar feb 22 20:29:26 -0300 2011:

Andrew Dunstan <andrew@dunslane.net> wrote:

Have you performance tested it? Scanning pg_index for index
columns for each row strikes me as likely to be unpleasant.

I haven't, yet. I had rather assumed that the index info for a
relation would have a high probability of being cached during
execution of an AFTER trigger for that relation, so I think we're
talking RAM access here. It didn't seem sane to try to create an
HTAB for this and worry about invalidation of it, etc. If there's a
faster way to get to the info without going to such extremes, I'd be
happy to hear them. (At least I avoided building and parsing a
query to get at it.)

I think it'd be better to use RelationGetIndexList (which gets the index
list from relcache) and fetch the index tuples from syscache; see
relationHasPrimaryKey for sample code.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#9Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Alvaro Herrera (#8)
1 attachment(s)
Re: How to extract a value from a record using attnum or attname?

Alvaro Herrera <alvherre@commandprompt.com> wrote:

I think it'd be better to use RelationGetIndexList (which gets the
index list from relcache) and fetch the index tuples from
syscache; see relationHasPrimaryKey for sample code.

Thanks. Patch done that way attached. Will get it into tomorrow's
system testing here.

-Kevin

Attachments:

tcn-2.patchtext/plain; name=tcn-2.patchDownload
*** a/src/backend/utils/adt/trigfuncs.c
--- b/src/backend/utils/adt/trigfuncs.c
***************
*** 13,21 ****
   */
  #include "postgres.h"
  
! #include "access/htup.h"
  #include "commands/trigger.h"
  #include "utils/builtins.h"
  
  
  /*
--- 13,23 ----
   */
  #include "postgres.h"
  
! #include "executor/spi.h"
! #include "commands/async.h"
  #include "commands/trigger.h"
  #include "utils/builtins.h"
+ #include "utils/syscache.h"
  
  
  /*
***************
*** 93,95 **** suppress_redundant_updates_trigger(PG_FUNCTION_ARGS)
--- 95,252 ----
  
  	return PointerGetDatum(rettuple);
  }
+ 
+ 
+ /*
+  * Copy from s (for source) to r (for result), wrapping with q (quote)
+  * characters and doubling any quote characters found.
+  */
+ static char *
+ strcpy_quoted(char *r, const char *s, const char q)
+ {
+ 	*r++ = q;
+ 	while (*s)
+ 	{
+ 		if (*s == q)
+ 			*r++ = q;
+ 		*r++ = *s;
+ 		s++;
+ 	}
+ 	*r++ = q;
+ 	return r;
+ }
+ 
+ /*
+  * triggered_change_notification
+  *
+  * This trigger function will send a notification of data modification with
+  * primary key values.	The channel will be "tcn" unless the trigger is
+  * created with a parameter, in which case that parameter will be used.
+  */
+ Datum
+ triggered_change_notification(PG_FUNCTION_ARGS)
+ {
+ 	TriggerData *trigdata = (TriggerData *) fcinfo->context;
+ 	Trigger    *trigger;
+ 	int			nargs;
+ 	HeapTuple	trigtuple;
+ 	Relation	rel;
+ 	TupleDesc	tupdesc;
+ 	char	   *channel;
+ 	char		operation;
+ 	char		payload[200];
+ 	char	   *p;
+ 	bool		foundPK;
+ 
+ 	List	   *indexoidlist;
+ 	ListCell   *indexoidscan;
+ 
+ 	/* make sure it's called as a trigger */
+ 	if (!CALLED_AS_TRIGGER(fcinfo))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 		errmsg("triggered_change_notification: must be called as trigger")));
+ 
+ 	/* and that it's called after the change */
+ 	if (!TRIGGER_FIRED_AFTER(trigdata->tg_event))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("triggered_change_notification: must be called after the change")));
+ 
+ 	/* and that it's called for each row */
+ 	if (!TRIGGER_FIRED_FOR_ROW(trigdata->tg_event))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("triggered_change_notification: must be called for each row")));
+ 
+ 	if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event))
+ 		operation = 'I';
+ 	else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
+ 		operation = 'U';
+ 	else if (TRIGGER_FIRED_BY_DELETE(trigdata->tg_event))
+ 		operation = 'D';
+ 	else
+ 	{
+ 		elog(ERROR, "triggered_change_notification: trigger fired by unrecognized operation");
+ 		operation = 'X';		/* silence compiler warning */
+ 	}
+ 
+ 	trigger = trigdata->tg_trigger;
+ 	nargs = trigger->tgnargs;
+ 	if (nargs > 1)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("triggered_change_notification: must not be called with more than one parameter")));
+ 
+ 	if (nargs == 0)
+ 		channel = "tcn";
+ 	else
+ 		channel = trigger->tgargs[0];
+ 
+ 	/* get tuple data */
+ 	trigtuple = trigdata->tg_trigtuple;
+ 	rel = trigdata->tg_relation;
+ 	tupdesc = rel->rd_att;
+ 
+ 	foundPK = false;
+ 
+ 	/*
+ 	 * Get the list of index OIDs for the table from the relcache, and look up
+ 	 * each one in the pg_index syscache until we find one marked primary key
+ 	 * (hopefully there isn't more than one such).
+ 	 */
+ 	indexoidlist = RelationGetIndexList(rel);
+ 
+ 	foreach(indexoidscan, indexoidlist)
+ 	{
+ 		Oid			indexoid = lfirst_oid(indexoidscan);
+ 		HeapTuple	indexTuple;
+ 		Form_pg_index index;
+ 
+ 		indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexoid));
+ 		if (!HeapTupleIsValid(indexTuple))		/* should not happen */
+ 			elog(ERROR, "cache lookup failed for index %u", indexoid);
+ 		index = (Form_pg_index) GETSTRUCT(indexTuple);
+ 		/* we're only interested if it is the primary key */
+ 		if (index->indisprimary)
+ 		{
+ 			int			numatts = index->indnatts;
+ 
+ 			if (numatts > 0)
+ 			{
+ 				int			i;
+ 
+ 				foundPK = true;
+ 
+ 				p = strcpy_quoted(payload, SPI_getrelname(rel), '"');
+ 				*p++ = ',';
+ 				*p++ = operation;
+ 
+ 				for (i = 0; i < numatts; i++)
+ 				{
+ 					int			colno = index->indkey.values[i];
+ 
+ 					*p++ = ',';
+ 					p = strcpy_quoted(p, SPI_fname(tupdesc, colno), '"');
+ 					*p++ = '=';
+ 					p = strcpy_quoted(p, SPI_getvalue(trigtuple, tupdesc, colno), '\'');
+ 				}
+ 				*p = '\0';
+ 
+ 				Async_Notify(channel, payload);
+ 			}
+ 			ReleaseSysCache(indexTuple);
+ 			break;
+ 		}
+ 		ReleaseSysCache(indexTuple);
+ 	}
+ 
+ 	list_free(indexoidlist);
+ 
+ 	if (!foundPK)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
+ 				 errmsg("triggered_change_notification: must be called on a table with a primary key")));
+ 
+ 	return PointerGetDatum(NULL);		/* after trigger; value doesn't matter */
+ }
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 1638,1643 **** DESCR("convert oid to int8");
--- 1638,1645 ----
  
  DATA(insert OID = 1291 (  suppress_redundant_updates_trigger	PGNSP PGUID 12 1 0 0 f f f t f v 0 0 2279 "" _null_ _null_ _null_ _null_ suppress_redundant_updates_trigger _null_ _null_ _null_ ));
  DESCR("trigger to suppress updates when new and old records match");
+ DATA(insert OID = 2650 (  triggered_change_notification	PGNSP PGUID 12 1 0 0 f f f t f v 0 0 2279 "" _null_ _null_ _null_ _null_ triggered_change_notification _null_ _null_ _null_ ));
+ DESCR("trigger function to send change notification with primary key in payload");
  
  DATA(insert OID = 1292 ( tideq			   PGNSP PGUID 12 1 0 0 f f f t f i 2 0 16 "27 27" _null_ _null_ _null_ _null_ tideq _null_ _null_ _null_ ));
  DESCR("equal");
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 963,968 **** extern Datum RI_FKey_setdefault_upd(PG_FUNCTION_ARGS);
--- 963,969 ----
  
  /* trigfuncs.c */
  extern Datum suppress_redundant_updates_trigger(PG_FUNCTION_ARGS);
+ extern Datum triggered_change_notification(PG_FUNCTION_ARGS);
  
  /* encoding support functions */
  extern Datum getdatabaseencoding(PG_FUNCTION_ARGS);
#10Alvaro Herrera
alvherre@commandprompt.com
In reply to: Kevin Grittner (#9)
Re: How to extract a value from a record using attnum or attname?

Excerpts from Kevin Grittner's message of mié feb 23 13:43:19 -0300 2011:

Alvaro Herrera <alvherre@commandprompt.com> wrote:

I think it'd be better to use RelationGetIndexList (which gets the
index list from relcache) and fetch the index tuples from
syscache; see relationHasPrimaryKey for sample code.

Thanks. Patch done that way attached. Will get it into tomorrow's
system testing here.

Why not use quote_identifier and quote_literal_cstr instead of this new
strcpy thing? Also, you don't really need spi.h do you?

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#11Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Alvaro Herrera (#10)
Re: How to extract a value from a record using attnum or attname?

Alvaro Herrera <alvherre@commandprompt.com> wrote:

Why not use quote_identifier and quote_literal_cstr instead of
this new strcpy thing?

We've got various types of software that will be parsing these
payloads, and it's a little easier to parse if the quoting is
unconditional. If that's a barrier to acceptance we could use
the functions which quote conditionally and adjust our regular
expressions.

Probably one reason we had a bias toward quoting is that every
single application table name we use has at least on uppercase
letter and about 95% of our column names do.

Also, you don't really need spi.h do you?

It's using these functions:

SPI_getrelname
SPI_fname
SPI_getvalue

If there's a better way to get the info, I'm game.

-Kevin

#12Alvaro Herrera
alvherre@commandprompt.com
In reply to: Kevin Grittner (#11)
Re: How to extract a value from a record using attnum or attname?

Excerpts from Kevin Grittner's message of mié feb 23 16:20:16 -0300 2011:

Alvaro Herrera <alvherre@commandprompt.com> wrote:

Why not use quote_identifier and quote_literal_cstr instead of
this new strcpy thing?

We've got various types of software that will be parsing these
payloads, and it's a little easier to parse if the quoting is
unconditional. If that's a barrier to acceptance we could use
the functions which quote conditionally and adjust our regular
expressions.

No strong opinion on this, really, but your strcpy should use a
StringInfo buffer instead of the char[200]. That's going to bite
someone.

Probably one reason we had a bias toward quoting is that every
single application table name we use has at least on uppercase
letter and about 95% of our column names do.

Makes sense.

Also, you don't really need spi.h do you?

It's using these functions:

SPI_getrelname
SPI_fname
SPI_getvalue

If there's a better way to get the info, I'm game.

I think you could get away without the first two (in particular get rid
of the memleak with SPI_getrelname), but the last one would require
something more involved. No strong opinion, I just failed to see those
calls in there.

Is this intended for 9.1?

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#13Robert Haas
robertmhaas@gmail.com
In reply to: Alvaro Herrera (#12)
Re: How to extract a value from a record using attnum or attname?

On Wed, Feb 23, 2011 at 2:48 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

Is this intended for 9.1?

Kevin already expressed his intention to add this to the first 9.2CF.
It's far too late to BEGIN discussing new features for 9.1.

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

#14Alvaro Herrera
alvherre@commandprompt.com
In reply to: Robert Haas (#13)
Re: How to extract a value from a record using attnum or attname?

Excerpts from Robert Haas's message of mié feb 23 17:03:23 -0300 2011:

On Wed, Feb 23, 2011 at 2:48 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

Is this intended for 9.1?

Kevin already expressed his intention to add this to the first 9.2CF.
It's far too late to BEGIN discussing new features for 9.1.

Yeah, I see that now. I'll go review some other patch then.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#15Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Alvaro Herrera (#12)
Re: How to extract a value from a record using attnum or attname?

Alvaro Herrera <alvherre@commandprompt.com> wrote:

Excerpts from Kevin Grittner's message:

No strong opinion on this, really, but your strcpy should use a
StringInfo buffer instead of the char[200]. That's going to bite
someone.

Yeah, this was thrown together in a bit of a hurry because of
development deadlines here, so I cut a few corners based on
knowledge of our particular implementation details. I know that's
something to change for general acceptance.

It's using these functions:

SPI_getrelname
SPI_fname
SPI_getvalue

If there's a better way to get the info, I'm game.

I think you could get away without the first two (in particular
get rid of the memleak with SPI_getrelname), but the last one
would require something more involved. No strong opinion, I just
failed to see those calls in there.

I thought the trigger would be running in a context which would make
that leak immaterial. I thought the general advice in such cases is
to *not* do retail freeing of space, but to let it get cleaned up
through release of the memory context. I'll take another look at
memory contexts around triggers.

Is this intended for 9.1?

Definitely not. I added it to the first 9.2 CF, as mentioned in
earlier posts. I was going to hold off posting until the beta was
wrapped, but it seemed reasonable to post the patch in response to
Dimitri's post. I wasn't intending to suggest it was ready for
general usage; I was mainly just putting it out there to see if
anyone was interested enough in it that I should polish it up for a
proper submission. For instance, there are no docs or regression
tests yet.

Anyway, I certainly appreciate the pointers, because we have to push
something out to production along these lines in a couple months to
stay on track with the organization's Annual Plan, which we need to
provide to the legislature and are judged against when they
authorize funding each year. I think your advice will bring this
feature from "it works" to "it works really well". :-)

-Kevin