Re: [HACKERS] 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 and...@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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to extract a value from a record using attnum or attname?
Alvaro Herrera alvhe...@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
*** 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;
+ Relationrel;
+ TupleDesc tupdesc;
+ char *channel;
+ charoperation;
+ charpayload[200];
+ char *p;
+ boolfoundPK;
+
+ 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);
+
Re: [HACKERS] 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 alvhe...@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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to extract a value from a record using attnum or attname?
Alvaro Herrera alvhe...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 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 alvhe...@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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to extract a value from a record using attnum or attname?
On Wed, Feb 23, 2011 at 2:48 PM, Alvaro Herrera alvhe...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 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 alvhe...@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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to extract a value from a record using attnum or attname?
Alvaro Herrera alvhe...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How to extract a value from a record using attnum or attname?
Andrew Dunstan and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
