Re: [HACKERS] minimal update
Kenneth Marshall wrote: On Wed, Oct 22, 2008 at 06:05:26PM -0400, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? I think it means something to us, but no op is a very technical phrase that probably doesn't travel very well. Agreed --- I was hoping someone could improve on that part. The only other words I could come up with were empty and useless, neither of which seem quite le mot juste ... regards, tom lane redundant? I think I like this best of all the suggestions - suppress_redundant_updates_trigger() is what I have now. If there's no further discussion, I'll go ahead and commit this in a day or two. cheers andrew ? GNUmakefile ? config.log ? config.status ? contrib/spi/.deps ? src/Makefile.global ? src/backend/postgres ? src/backend/access/common/.deps ? src/backend/access/gin/.deps ? src/backend/access/gist/.deps ? src/backend/access/hash/.deps ? src/backend/access/heap/.deps ? src/backend/access/index/.deps ? src/backend/access/nbtree/.deps ? src/backend/access/transam/.deps ? src/backend/bootstrap/.deps ? src/backend/catalog/.deps ? src/backend/catalog/postgres.bki ? src/backend/catalog/postgres.description ? src/backend/catalog/postgres.shdescription ? src/backend/commands/.deps ? src/backend/executor/.deps ? src/backend/lib/.deps ? src/backend/libpq/.deps ? src/backend/main/.deps ? src/backend/nodes/.deps ? src/backend/optimizer/geqo/.deps ? src/backend/optimizer/path/.deps ? src/backend/optimizer/plan/.deps ? src/backend/optimizer/prep/.deps ? src/backend/optimizer/util/.deps ? src/backend/parser/.deps ? src/backend/port/.deps ? src/backend/postmaster/.deps ? src/backend/regex/.deps ? src/backend/rewrite/.deps ? src/backend/snowball/.deps ? src/backend/snowball/snowball_create.sql ? src/backend/storage/buffer/.deps ? src/backend/storage/file/.deps ? src/backend/storage/freespace/.deps ? src/backend/storage/ipc/.deps ? src/backend/storage/large_object/.deps ? src/backend/storage/lmgr/.deps ? src/backend/storage/page/.deps ? src/backend/storage/smgr/.deps ? src/backend/tcop/.deps ? src/backend/tsearch/.deps ? src/backend/utils/.deps ? src/backend/utils/probes.h ? src/backend/utils/adt/.deps ? src/backend/utils/cache/.deps ? src/backend/utils/error/.deps ? src/backend/utils/fmgr/.deps ? src/backend/utils/hash/.deps ? src/backend/utils/init/.deps ? src/backend/utils/mb/.deps ? src/backend/utils/mb/conversion_procs/conversion_create.sql ? src/backend/utils/mb/conversion_procs/ascii_and_mic/.deps ? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/.deps ? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/.deps ? src/backend/utils/mb/conversion_procs/euc_jis_2004_and_shift_jis_2004/.deps ? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/.deps ? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/.deps ? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/.deps ? src/backend/utils/mb/conversion_procs/latin2_and_win1250/.deps ? src/backend/utils/mb/conversion_procs/latin_and_mic/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_ascii/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_big5/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_euc_jis_2004/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_gbk/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_johab/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_shift_jis_2004/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_sjis/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_uhc/.deps ? src/backend/utils/mb/conversion_procs/utf8_and_win/.deps ? src/backend/utils/misc/.deps ? src/backend/utils/mmgr/.deps ? src/backend/utils/resowner/.deps ? src/backend/utils/sort/.deps ? src/backend/utils/time/.deps ? src/bin/initdb/.deps ? src/bin/initdb/initdb ? src/bin/pg_config/.deps ? src/bin/pg_config/pg_config ? src/bin/pg_controldata/.deps ? src/bin/pg_controldata/pg_controldata ? src/bin/pg_ctl/.deps ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/.deps ? src/bin/pg_dump/pg_dump ? src/bin/pg_dump/pg_dumpall ? src/bin/pg_dump/pg_restore ? src/bin/pg_resetxlog/.deps ? src/bin/pg_resetxlog/pg_resetxlog ? src/bin/psql/.deps ? src/bin/psql/psql ? src/bin/scripts/.deps ? src/bin/scripts/clusterdb ? src/bin/scripts/createdb ? src/bin/scripts/createlang ?
Re: [HACKERS] minimal update
Andrew Dunstan wrote: Kenneth Marshall wrote: On Wed, Oct 22, 2008 at 06:05:26PM -0400, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? I think it means something to us, but no op is a very technical phrase that probably doesn't travel very well. Agreed --- I was hoping someone could improve on that part. The only other words I could come up with were empty and useless, neither of which seem quite le mot juste ... regards, tom lane redundant? I think I like this best of all the suggestions - suppress_redundant_updates_trigger() is what I have now. If there's no further discussion, I'll go ahead and commit this in a day or two. Nitpicking, but you have: +para + Currently productnamePostgreSQL/ provides one built in trigger + function, functionsuppress_redundant_updates_trigger/, Should we perhaps mention the fulltext triggers (with the appropriate links) here? If it's intended to be an authoritative list of the userspace triggers we ship, I think that may be a good idea. //Magnus -- 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] minimal update
Andrew Dunstan escribió: + /* make sure it's called as a trigger */ + if (!CALLED_AS_TRIGGER(fcinfo)) + elog(ERROR, suppress_redundant_updates_trigger: must be called as trigger); Shouldn't these all be ereport()? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] minimal update
Alvaro Herrera wrote: Andrew Dunstan escribió: + /* make sure it's called as a trigger */ + if (!CALLED_AS_TRIGGER(fcinfo)) + elog(ERROR, suppress_redundant_updates_trigger: must be called as trigger); Shouldn't these all be ereport()? Good point. I'll fix them. Maybe we should fix our C sample trigger, from which this was taken. cheers andrew -- 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] minimal update
On Wed, Oct 29, 2008 at 03:48:09PM -0400, Andrew Dunstan wrote: + /* make sure it's called as a trigger */ + if (!CALLED_AS_TRIGGER(fcinfo)) + elog(ERROR, suppress_redundant_updates_trigger: must be called as trigger); Shouldn't these all be ereport()? Good point. I'll fix them. Maybe we should fix our C sample trigger, from which this was taken. Yes :) Does the attached have the right error code? Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index a3f17c9..69430ea 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -581,7 +581,9 @@ trigf(PG_FUNCTION_ARGS) /* make sure it's called as a trigger at all */ if (!CALLED_AS_TRIGGER(fcinfo)) -elog(ERROR, trigf: not called by trigger manager); +ereport(ERROR, +(error(TRIGGERED_DATA_CHANGE_VIOLATION), + errmsg(trigf: not called by trigger manager))); /* tuple to return to executor */ if (TRIGGER_FIRED_BY_UPDATE(trigdata-gt;tg_event)) -- 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] minimal update
David Fetter wrote: Maybe we should fix our C sample trigger, from which this was taken. Yes :) Does the attached have the right error code? -elog(ERROR, trigf: not called by trigger manager); +ereport(ERROR, +(error(TRIGGERED_DATA_CHANGE_VIOLATION), + errmsg(trigf: not called by trigger manager))); Not sure that's appropriate, but I can't see anything else that is very appropriate either. cheers andrew -- 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] minimal update
Andrew Dunstan wrote: David Fetter wrote: Maybe we should fix our C sample trigger, from which this was taken. Yes :) Does the attached have the right error code? -elog(ERROR, trigf: not called by trigger manager); +ereport(ERROR, +(error(TRIGGERED_DATA_CHANGE_VIOLATION), + errmsg(trigf: not called by trigger manager))); Not sure that's appropriate, but I can't see anything else that is very appropriate either. The plpgsql code uses errcode(ERRCODE_FEATURE_NOT_SUPPORTED) for this situation, so I guess we should be consistent with that. cheers andrew -- 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] minimal update
Andrew Dunstan [EMAIL PROTECTED] writes: Not sure that's appropriate, but I can't see anything else that is very appropriate either. The plpgsql code uses errcode(ERRCODE_FEATURE_NOT_SUPPORTED) for this situation, so I guess we should be consistent with that. TRIGGERED_DATA_CHANGE_VIOLATION is most certainly NOT an appropriate code here --- it's talking about invalid database content states. The RI triggers use ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED for these sorts of conditions, and I think that's probably best practice. See ri_CheckTrigger() in particular. regards, tom lane -- 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] minimal update
Andrew Dunstan [EMAIL PROTECTED] writes: I think I like this best of all the suggestions - suppress_redundant_updates_trigger() is what I have now. If there's no further discussion, I'll go ahead and commit this in a day or two. The documentation seems a bit lacking: it gives neither a hint of why you might want to use this nor why it's not the built-in behavior. Suggest expending a sentence or two pointing out that the trigger takes nonzero execution time to do its comparisons, and that this may or may not be repaid by eliminated updates, depending on whether the client applications are actually in the habit of issuing useless update commands. I think you're missing an indexentry item for the function name, also. regards, tom lane -- 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] minimal update
On Oct 22, 2008, at 1:43 PM, Andrew Dunstan wrote: + if (!CALLED_AS_TRIGGER(fcinfo)) + elog(ERROR, min_update_trigger: not called by trigger manager); The error I get in 8.2 when calling a trigger function directly is: ERROR: trigger functions may only be called as triggers To stay consistent, I think the remaining errors should s/: not/ may only be/, ie: min_update_trigger may only be called on update + /* and that it's called on update */ + if (! TRIGGER_FIRED_BY_UPDATE(trigdata-tg_event)) + elog(ERROR, min_update_trigger: not called on update); + + /* and that it's called before update */ + if (! TRIGGER_FIRED_BEFORE(trigdata-tg_event)) + elog(ERROR, min_update_trigger: not called before update); + + /* and that it's called for each row */ + if (! TRIGGER_FIRED_FOR_ROW(trigdata-tg_event)) + elog(ERROR, min_update_trigger: not called for each row); -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] minimal update
Andrew Dunstan wrote:
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
In that case, why not put the trigger in core so people can use it
easily?
One advantage of making it a contrib module is that discussing how/when
to use it would fit more easily into the structure of the
documentation. There is no place in our docs that a standard trigger
would fit without seeming like a wart; but a contrib module can document
itself pretty much however it wants.
I was thinking a new section on 'trigger functions' of the functions
and operators chapter, linked from the 'create trigger' page. That
doesn't seem like too much of a wart.
There seems to be a preponderance of opinion for doing this as a
builtin. Here is a patch that does it that way, along with docs and
regression test.
cheers
andrew
Index: doc/src/sgml/func.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.450
diff -c -r1.450 func.sgml
*** doc/src/sgml/func.sgml 14 Oct 2008 17:12:32 - 1.450
--- doc/src/sgml/func.sgml 22 Oct 2008 18:35:51 -
***
*** 12817,12820
--- 12817,12845
/sect1
+ sect1 id=functions-trigger
+titleTrigger Functions/title
+
+para
+ Currently productnamePostgreSQL/ provides one built in trigger
+ function, functionmin_update_trigger/, which will prevent any update
+ that does not actually change the data in the row from taking place, in
+ contrast to the normal behaviour which always performs the update
+ regardless of whether or not the data has changed.
+ /para
+
+ para
+ The functionmin_update_trigger/ function can be added to a table
+ like this:
+ programlisting
+ CREATE TRIGGER _min_update
+ BEFORE UPDATE ON tablename
+ FOR EACH ROW EXECUTE PROCEDURE min_update_trigger();
+ /programlisting
+ /para
+ para
+For mare information about creating triggers, see
+ xref linkend=SQL-CREATETRIGGER.
+ /para
+ /sect1
/chapter
Index: src/backend/utils/adt/Makefile
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/Makefile,v
retrieving revision 1.69
diff -c -r1.69 Makefile
*** src/backend/utils/adt/Makefile 19 Feb 2008 10:30:08 - 1.69
--- src/backend/utils/adt/Makefile 22 Oct 2008 18:35:51 -
***
*** 25,31
tid.o timestamp.o varbit.o varchar.o varlena.o version.o xid.o \
network.o mac.o inet_net_ntop.o inet_net_pton.o \
ri_triggers.o pg_lzcompress.o pg_locale.o formatting.o \
! ascii.o quote.o pgstatfuncs.o encode.o dbsize.o genfile.o \
tsginidx.o tsgistidx.o tsquery.o tsquery_cleanup.o tsquery_gist.o \
tsquery_op.o tsquery_rewrite.o tsquery_util.o tsrank.o \
tsvector.o tsvector_op.o tsvector_parser.o \
--- 25,31
tid.o timestamp.o varbit.o varchar.o varlena.o version.o xid.o \
network.o mac.o inet_net_ntop.o inet_net_pton.o \
ri_triggers.o pg_lzcompress.o pg_locale.o formatting.o \
! ascii.o quote.o pgstatfuncs.o encode.o dbsize.o genfile.o trigfuncs.o \
tsginidx.o tsgistidx.o tsquery.o tsquery_cleanup.o tsquery_gist.o \
tsquery_op.o tsquery_rewrite.o tsquery_util.o tsrank.o \
tsvector.o tsvector_op.o tsvector_parser.o \
Index: src/backend/utils/adt/trigfuncs.c
===
RCS file: src/backend/utils/adt/trigfuncs.c
diff -N src/backend/utils/adt/trigfuncs.c
*** /dev/null 1 Jan 1970 00:00:00 -
--- src/backend/utils/adt/trigfuncs.c 22 Oct 2008 18:35:51 -
***
*** 0
--- 1,73
+ /*-
+ *
+ * trigfuncs.c
+ *Builtin functions for useful trigger support.
+ *
+ *
+ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * $PostgreSQL:$
+ *
+ *-
+ */
+
+
+
+ #include postgres.h
+ #include commands/trigger.h
+ #include access/htup.h
+
+ /*
+ * min_update_trigger
+ *
+ * This trigger function will inhibit an update from being done
+ * if the OLD and NEW records are identical.
+ *
+ */
+
+ Datum
+ min_update_trigger(PG_FUNCTION_ARGS)
+ {
+ TriggerData *trigdata = (TriggerData *) fcinfo-context;
+ HeapTuple newtuple, oldtuple, rettuple;
+ HeapTupleHeader newheader, oldheader;
+
+ /* make sure it's called as a trigger */
+ if (!CALLED_AS_TRIGGER(fcinfo))
+ elog(ERROR, min_update_trigger: not called by trigger manager);
+
+ /* and that it's called on update */
+ if (! TRIGGER_FIRED_BY_UPDATE(trigdata-tg_event))
+ elog(ERROR, min_update_trigger: not called on update);
+
+ /* and that it's called before update */
+ if (! TRIGGER_FIRED_BEFORE(trigdata-tg_event))
+ elog(ERROR,
Re: [HACKERS] minimal update
Andrew Dunstan [EMAIL PROTECTED] wrote: Here is a patch that does it that way, along with docs s/mare/more/ -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] minimal update
Kevin Grittner wrote: Andrew Dunstan [EMAIL PROTECTED] wrote: Here is a patch that does it that way, along with docs s/mare/more/ Thanks. fixed in my tree.. cheers andrew -- 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] minimal update
On Wed, 2008-10-22 at 14:43 -0400, Andrew Dunstan wrote: There seems to be a preponderance of opinion for doing this as a builtin. Here is a patch that does it that way, along with docs and regression test. In your example you use an underscore as the first character. The way you have done this it will probably exclude any other before row triggers from firing, which may have altered the value of one or more columns. The more probable choice for me would be to have a trigger that came after all other before triggers, and so should have a different name. It's just an example, so your choice is fine, but I think you should bring out that point more clearly for the average developer. Can we call the function minimal_update_trigger, rather than min_... -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and 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] minimal update
Simon Riggs [EMAIL PROTECTED] writes: Can we call the function minimal_update_trigger, rather than min_... Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? regards, tom lane -- 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] minimal update
Simon Riggs wrote: On Wed, 2008-10-22 at 14:43 -0400, Andrew Dunstan wrote: There seems to be a preponderance of opinion for doing this as a builtin. Here is a patch that does it that way, along with docs and regression test. In your example you use an underscore as the first character. The way you have done this it will probably exclude any other before row triggers from firing, which may have altered the value of one or more columns. The more probable choice for me would be to have a trigger that came after all other before triggers, and so should have a different name. It's just an example, so your choice is fine, but I think you should bring out that point more clearly for the average developer. Fair point. I'll add that to the docs. Can we call the function minimal_update_trigger, rather than min_... If that's the general consensus, sure. I have no strong opinion. cheers andrew -- 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] minimal update
On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: Simon Riggs [EMAIL PROTECTED] writes: Can we call the function minimal_update_trigger, rather than min_... Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? +1. That's a much better name. ...Robert -- 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] minimal update
On Wed, 2008-10-22 at 17:24 -0400, Robert Haas wrote: On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: Simon Riggs [EMAIL PROTECTED] writes: Can we call the function minimal_update_trigger, rather than min_... Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? +1. That's a much better name. I think it means something to us, but no op is a very technical phrase that probably doesn't travel very well. Not everybody Majored in Comp Sci and speaks Amglish as their native language. Certainly this intention is much better than minimal, but a more widely acceptable phrase is probably better. I will avoid trying to come up with something myself though. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and 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] minimal update
Simon Riggs [EMAIL PROTECTED] writes: On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? I think it means something to us, but no op is a very technical phrase that probably doesn't travel very well. Agreed --- I was hoping someone could improve on that part. The only other words I could come up with were empty and useless, neither of which seem quite le mot juste ... regards, tom lane -- 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] minimal update
On Wed, Oct 22, 2008 at 06:05:26PM -0400, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: Minimal really fails to convey the point here IMHO. How about something like suppress_no_op_updates_trigger? I think it means something to us, but no op is a very technical phrase that probably doesn't travel very well. Agreed --- I was hoping someone could improve on that part. The only other words I could come up with were empty and useless, neither of which seem quite le mot juste ... regards, tom lane redundant? Ken -- 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] minimal update
Simon Riggs [EMAIL PROTECTED] wrote: On Wed, Oct 22, 2008 at 3:24 PM, Tom Lane [EMAIL PROTECTED] wrote: How about something like suppress_no_op_updates_trigger? I think it means something to us, but no op is a very technical phrase that probably doesn't travel very well. Not everybody Majored in Comp Sci and speaks Amglish as their native language. Certainly this intention is much better than minimal, but a more widely acceptable phrase is probably better. I will avoid trying to come up with something myself though. How about one of these?: suppress_same_value_updates_trigger suppress_no_change_updates_trigger suppress_no_effect_updates_trigger They all seem a bit awkward, but the best that came to mind. -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] minimal update
How about one of these?: suppress_same_value_updates_trigger suppress_no_change_updates_trigger suppress_no_effect_updates_trigger I like the first one. A trigger firing would be an effect, and possibly a change, but same value seems very clear. ...Robert -- 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] minimal update
On 20 okt 2008, at 16.51, Andrew Dunstan [EMAIL PROTECTED] wrote: Magnus Hagander wrote: Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? I thought the plan was to make it a contrib module. Well, previous discussion did mention catalog entries, which would suggest otherwise, but I can do it as a contrib module if that's the consensus. What would be the actual reason to put it in contrib and not core? Are there any dangers by having it there? Or is it just a hack and not a real solution? No, it's not just a hack. It's very close to what we'd probably do if we built the facility right into the language, although it does involve the overhead of calling the trigger. However, it performs reasonably well - not surprising since the guts of it is just a memcmp() call. In that case, why not put the trigger in core so people can use it easily? /magnus -- 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] minimal update
On Tue, Oct 21, 2008 at 03:34:04PM +0200, Magnus Hagander wrote: On 20 okt 2008, at 16.51, Andrew Dunstan [EMAIL PROTECTED] wrote: Magnus Hagander wrote: Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? I thought the plan was to make it a contrib module. Well, previous discussion did mention catalog entries, which would suggest otherwise, but I can do it as a contrib module if that's the consensus. What would be the actual reason to put it in contrib and not core? Are there any dangers by having it there? Or is it just a hack and not a real solution? No, it's not just a hack. It's very close to what we'd probably do if we built the facility right into the language, although it does involve the overhead of calling the trigger. However, it performs reasonably well - not surprising since the guts of it is just a memcmp() call. In that case, why not put the trigger in core so people can use it easily? +1 :) Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] minimal update
On Tue, Oct 21, 2008 at 9:34 AM, Magnus Hagander [EMAIL PROTECTED] wrote: On 20 okt 2008, at 16.51, Andrew Dunstan [EMAIL PROTECTED] wrote: No, it's not just a hack. It's very close to what we'd probably do if we built the facility right into the language, although it does involve the overhead of calling the trigger. However, it performs reasonably well - not surprising since the guts of it is just a memcmp() call. In that case, why not put the trigger in core so people can use it easily? +1 This is hard to get right and a common source of errors. merlin -- 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] minimal update
Magnus Hagander [EMAIL PROTECTED] writes: In that case, why not put the trigger in core so people can use it easily? One advantage of making it a contrib module is that discussing how/when to use it would fit more easily into the structure of the documentation. There is no place in our docs that a standard trigger would fit without seeming like a wart; but a contrib module can document itself pretty much however it wants. regards, tom lane -- 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] minimal update
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: In that case, why not put the trigger in core so people can use it easily? One advantage of making it a contrib module is that discussing how/when to use it would fit more easily into the structure of the documentation. There is no place in our docs that a standard trigger would fit without seeming like a wart; but a contrib module can document itself pretty much however it wants. I was thinking a new section on 'trigger functions' of the functions and operators chapter, linked from the 'create trigger' page. That doesn't seem like too much of a wart. cheers andrew -- 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] minimal update
Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? I thought the plan was to make it a contrib module. Well, previous discussion did mention catalog entries, which would suggest otherwise, but I can do it as a contrib module if that's the consensus. What would be the actual reason to put it in contrib and not core? Are there any dangers by having it there? Or is it just a hack and not a real solution? //Magnus -- 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] minimal update
Magnus Hagander wrote: Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? I thought the plan was to make it a contrib module. Well, previous discussion did mention catalog entries, which would suggest otherwise, but I can do it as a contrib module if that's the consensus. What would be the actual reason to put it in contrib and not core? Are there any dangers by having it there? Or is it just a hack and not a real solution? No, it's not just a hack. It's very close to what we'd probably do if we built the facility right into the language, although it does involve the overhead of calling the trigger. However, it performs reasonably well - not surprising since the guts of it is just a memcmp() call. cheers andrew -- 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] minimal update
Bruce Momjian wrote: Andrew Dunstan wrote: Bruce, did you ever look at completing this? No, it is still in my email box unaddressed. Feel free to work on it; I doubt I can do it for 8.4. OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? cheers andrew -- 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] minimal update
Andrew Dunstan [EMAIL PROTECTED] writes: OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? I thought the plan was to make it a contrib module. regards, tom lane -- 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] minimal update
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: OK. Where would be a good place to put the code? Maybe a new file src/backend/utils/adt/trigger_utils.c ? I thought the plan was to make it a contrib module. Well, previous discussion did mention catalog entries, which would suggest otherwise, but I can do it as a contrib module if that's the consensus. cheers andrew -- 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] minimal update
Andrew Dunstan wrote:
Bruce,
did you ever look at completing this?
No, it is still in my email box unaddressed. Feel free to work on it; I
doubt I can do it for 8.4.
---
cheers
andrew
Andrew Dunstan wrote:
Bruce Momjian wrote:
Andrew Dunstan wrote:
Right. In fact, I already had that part in fact - see
http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
What I was waiting for was the part where it gets put in the
catalog, documented, etc.
I can probably do that part. Send over what you have and I will work on
it. Thanks.
It's very similar to what Gurjeet posted (but designed to work with
earlier postgres versions)
cheers
andrew
---
|#include postgres.h
#include commands/trigger.h
#include access/htup.h
#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif
/* for pre 8.3 */
#ifndef HeapTupleHeaderGetNatts
#define HeapTupleHeaderGetNatts(th) ((th)-t_natts )
#endif
extern Datum min_update_trigger(PG_FUNCTION_ARGS);
PG_FUNCTION_INFO_V1(min_update_trigger);
Datum
min_update_trigger(PG_FUNCTION_ARGS)
{
TriggerData *trigdata = (TriggerData *) fcinfo-context;
HeapTuple newtuple, oldtuple, rettuple;
/* make sure it's called as a trigger at all */
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, min_update_trigger: not called by trigger manager);
/* and that it's called on update */
if (! TRIGGER_FIRED_BY_UPDATE(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called on update);
/* and that it's called before update */
if (! TRIGGER_FIRED_BEFORE(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called before update);
/* and that it's called for each row */
if (! TRIGGER_FIRED_FOR_ROW(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called for each row);
/* get tuple dat, set default return */
rettuple = newtuple = trigdata-tg_newtuple;
oldtuple = trigdata-tg_trigtuple;
if (newtuple-t_len == oldtuple-t_len
newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newtuple-t_data) ==
HeapTupleHeaderGetNatts(oldtuple-t_data)
(newtuple-t_data-t_infomask ~HEAP_XACT_MASK) ==
(oldtuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp(((char *)newtuple-t_data) +
offsetof(HeapTupleHeaderData, t_bits),
((char *)oldtuple-t_data) +
offsetof(HeapTupleHeaderData, t_bits),
newtuple-t_len -
offsetof(HeapTupleHeaderData, t_bits)) == 0)
rettuple = NULL;
return PointerGetDatum(rettuple);
}|
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
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] minimal update
Bruce,
did you ever look at completing this?
cheers
andrew
Andrew Dunstan wrote:
Bruce Momjian wrote:
Andrew Dunstan wrote:
Right. In fact, I already had that part in fact - see
http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
What I was waiting for was the part where it gets put in the
catalog, documented, etc.
I can probably do that part. Send over what you have and I will work on
it. Thanks.
It's very similar to what Gurjeet posted (but designed to work with
earlier postgres versions)
cheers
andrew
---
|#include postgres.h
#include commands/trigger.h
#include access/htup.h
#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif
/* for pre 8.3 */
#ifndef HeapTupleHeaderGetNatts
#define HeapTupleHeaderGetNatts(th) ((th)-t_natts )
#endif
extern Datum min_update_trigger(PG_FUNCTION_ARGS);
PG_FUNCTION_INFO_V1(min_update_trigger);
Datum
min_update_trigger(PG_FUNCTION_ARGS)
{
TriggerData *trigdata = (TriggerData *) fcinfo-context;
HeapTuple newtuple, oldtuple, rettuple;
/* make sure it's called as a trigger at all */
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, min_update_trigger: not called by trigger manager);
/* and that it's called on update */
if (! TRIGGER_FIRED_BY_UPDATE(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called on update);
/* and that it's called before update */
if (! TRIGGER_FIRED_BEFORE(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called before update);
/* and that it's called for each row */
if (! TRIGGER_FIRED_FOR_ROW(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called for each row);
/* get tuple dat, set default return */
rettuple = newtuple = trigdata-tg_newtuple;
oldtuple = trigdata-tg_trigtuple;
if (newtuple-t_len == oldtuple-t_len
newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newtuple-t_data) ==
HeapTupleHeaderGetNatts(oldtuple-t_data)
(newtuple-t_data-t_infomask ~HEAP_XACT_MASK) ==
(oldtuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp(((char *)newtuple-t_data) +
offsetof(HeapTupleHeaderData, t_bits),
((char *)oldtuple-t_data) +
offsetof(HeapTupleHeaderData, t_bits),
newtuple-t_len -
offsetof(HeapTupleHeaderData, t_bits)) == 0)
rettuple = NULL;
return PointerGetDatum(rettuple);
}|
--
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] minimal update
Is there a version of this patch ready for application?
---
Gurjeet Singh wrote:
On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan [EMAIL PROTECTED] wrote:
Gurjeet Singh wrote:
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
I assume don't want a TODO for this? (Suppress UPDATE no changed
columns)
I am starting to implement this. Do we want to have this trigger
function in the server, or in an external module?
I have the trigger part of this done, in fact. What remains to be done
is to add it to the catalog and document it. The intention is to make it
a builtin as it will be generally useful. If you want to work on the
remaining parts then I will happily ship you the C code for the trigger.
In fact, I just finished writing the C code and including it in the catalog
(Just tested that it's visible in the catalog). I will test it to see if it
does actually do what we want it to.
I have incorporated all the suggestions above. Would love to see your code
in the meantime.
Here's the C code:
Datum
trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
{
TriggerData *trigData;
HeapTuple oldTuple;
HeapTuple newTuple;
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, trig_ignore_duplicate_updates: not called by trigger
manager.);
if( !TRIGGER_FIRED_BY_UPDATE(trigData-tg_event)
!TRIGGER_FIRED_BEFORE(trigData-tg_event)
!TRIGGER_FIRED_FOR_ROW(trigData-tg_event) )
{
elog(ERROR, trig_ignore_duplicate_updates: Can only be executed for
UPDATE, BEFORE and FOR EACH ROW.);
}
trigData = (TriggerData *) fcinfo-context;
oldTuple = trigData-tg_trigtuple;
newTuple = trigData-tg_newtuple;
if (newTuple-t_len == oldTuple-t_len
newTuple-t_data-t_hoff == oldTuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newTuple-t_data) ==
HeapTupleHeaderGetNatts(oldTuple-t_data)
(newTuple-t_data-t_infomask ~HEAP_XACT_MASK)
== (oldTuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp( (char*)(newTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
(char*)(oldTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newTuple-t_len - offsetof(HeapTupleHeaderData, t_bits)
) == 0 )
{
/* return without crating a new tuple */
return PointerGetDatum( NULL );
}
return PointerGetDatum( trigData-tg_newtuple );
}
--
[EMAIL PROTECTED]
[EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17? 29' 34.37N, 78? 30' 59.76E - Hyderabad *
18? 32' 57.25N, 73? 56' 25.42E - Pune
37? 47' 19.72N, 122? 24' 1.69 W - San Francisco
http://gurjeet.frihost.net
Mail sent from my BlackLaptop device
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
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] minimal update
Not that I know of. I never saw Gurjeet's completed code.
cheers
andrew
Bruce Momjian wrote:
Is there a version of this patch ready for application?
---
Gurjeet Singh wrote:
On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan [EMAIL PROTECTED] wrote:
Gurjeet Singh wrote:
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
I assume don't want a TODO for this? (Suppress UPDATE no changed
columns)
I am starting to implement this. Do we want to have this trigger
function in the server, or in an external module?
I have the trigger part of this done, in fact. What remains to be done
is to add it to the catalog and document it. The intention is to make it
a builtin as it will be generally useful. If you want to work on the
remaining parts then I will happily ship you the C code for the trigger.
In fact, I just finished writing the C code and including it in the catalog
(Just tested that it's visible in the catalog). I will test it to see if it
does actually do what we want it to.
I have incorporated all the suggestions above. Would love to see your code
in the meantime.
Here's the C code:
Datum
trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
{
TriggerData *trigData;
HeapTuple oldTuple;
HeapTuple newTuple;
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, trig_ignore_duplicate_updates: not called by trigger
manager.);
if( !TRIGGER_FIRED_BY_UPDATE(trigData-tg_event)
!TRIGGER_FIRED_BEFORE(trigData-tg_event)
!TRIGGER_FIRED_FOR_ROW(trigData-tg_event) )
{
elog(ERROR, trig_ignore_duplicate_updates: Can only be executed for
UPDATE, BEFORE and FOR EACH ROW.);
}
trigData = (TriggerData *) fcinfo-context;
oldTuple = trigData-tg_trigtuple;
newTuple = trigData-tg_newtuple;
if (newTuple-t_len == oldTuple-t_len
newTuple-t_data-t_hoff == oldTuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newTuple-t_data) ==
HeapTupleHeaderGetNatts(oldTuple-t_data)
(newTuple-t_data-t_infomask ~HEAP_XACT_MASK)
== (oldTuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp( (char*)(newTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
(char*)(oldTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newTuple-t_len - offsetof(HeapTupleHeaderData, t_bits)
) == 0 )
{
/* return without crating a new tuple */
return PointerGetDatum( NULL );
}
return PointerGetDatum( trigData-tg_newtuple );
}
--
[EMAIL PROTECTED]
[EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17? 29' 34.37N, 78? 30' 59.76E - Hyderabad *
18? 32' 57.25N, 73? 56' 25.42E - Pune
37? 47' 19.72N, 122? 24' 1.69 W - San Francisco
http://gurjeet.frihost.net
Mail sent from my BlackLaptop device
--
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] minimal update
Andrew Dunstan wrote:
Not that I know of. I never saw Gurjeet's completed code.
This is Gurjeet's code, but it is not complete.
http://archives.postgresql.org/pgsql-hackers/2008-03/msg00668.php
---
cheers
andrew
Bruce Momjian wrote:
Is there a version of this patch ready for application?
---
Gurjeet Singh wrote:
On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan [EMAIL PROTECTED] wrote:
Gurjeet Singh wrote:
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
I assume don't want a TODO for this? (Suppress UPDATE no changed
columns)
I am starting to implement this. Do we want to have this trigger
function in the server, or in an external module?
I have the trigger part of this done, in fact. What remains to be done
is to add it to the catalog and document it. The intention is to make it
a builtin as it will be generally useful. If you want to work on the
remaining parts then I will happily ship you the C code for the trigger.
In fact, I just finished writing the C code and including it in the catalog
(Just tested that it's visible in the catalog). I will test it to see if it
does actually do what we want it to.
I have incorporated all the suggestions above. Would love to see your code
in the meantime.
Here's the C code:
Datum
trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
{
TriggerData *trigData;
HeapTuple oldTuple;
HeapTuple newTuple;
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, trig_ignore_duplicate_updates: not called by trigger
manager.);
if( !TRIGGER_FIRED_BY_UPDATE(trigData-tg_event)
!TRIGGER_FIRED_BEFORE(trigData-tg_event)
!TRIGGER_FIRED_FOR_ROW(trigData-tg_event) )
{
elog(ERROR, trig_ignore_duplicate_updates: Can only be executed
for
UPDATE, BEFORE and FOR EACH ROW.);
}
trigData = (TriggerData *) fcinfo-context;
oldTuple = trigData-tg_trigtuple;
newTuple = trigData-tg_newtuple;
if (newTuple-t_len == oldTuple-t_len
newTuple-t_data-t_hoff == oldTuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newTuple-t_data) ==
HeapTupleHeaderGetNatts(oldTuple-t_data)
(newTuple-t_data-t_infomask ~HEAP_XACT_MASK)
== (oldTuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp( (char*)(newTuple-t_data) +
offsetof(HeapTupleHeaderData,
t_bits),
(char*)(oldTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newTuple-t_len - offsetof(HeapTupleHeaderData, t_bits)
) == 0 )
{
/* return without crating a new tuple */
return PointerGetDatum( NULL );
}
return PointerGetDatum( trigData-tg_newtuple );
}
--
[EMAIL PROTECTED]
[EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17? 29' 34.37N, 78? 30' 59.76E - Hyderabad *
18? 32' 57.25N, 73? 56' 25.42E - Pune
37? 47' 19.72N, 122? 24' 1.69 W - San Francisco
http://gurjeet.frihost.net
Mail sent from my BlackLaptop device
--
Bruce Momjian [EMAIL PROTECTED]http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
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] minimal update
Right. In fact, I already had that part in fact - see
http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
What I was waiting for was the part where it gets put in the catalog,
documented, etc.
cheers
andrew
Bruce Momjian wrote:
Andrew Dunstan wrote:
Not that I know of. I never saw Gurjeet's completed code.
This is Gurjeet's code, but it is not complete.
http://archives.postgresql.org/pgsql-hackers/2008-03/msg00668.php
---
cheers
andrew
Bruce Momjian wrote:
Is there a version of this patch ready for application?
---
Gurjeet Singh wrote:
On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan [EMAIL PROTECTED] wrote:
Gurjeet Singh wrote:
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
I assume don't want a TODO for this? (Suppress UPDATE no changed
columns)
I am starting to implement this. Do we want to have this trigger
function in the server, or in an external module?
I have the trigger part of this done, in fact. What remains to be done
is to add it to the catalog and document it. The intention is to make it
a builtin as it will be generally useful. If you want to work on the
remaining parts then I will happily ship you the C code for the trigger.
In fact, I just finished writing the C code and including it in the catalog
(Just tested that it's visible in the catalog). I will test it to see if it
does actually do what we want it to.
I have incorporated all the suggestions above. Would love to see your code
in the meantime.
Here's the C code:
Datum
trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
{
TriggerData *trigData;
HeapTuple oldTuple;
HeapTuple newTuple;
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, trig_ignore_duplicate_updates: not called by trigger
manager.);
if( !TRIGGER_FIRED_BY_UPDATE(trigData-tg_event)
!TRIGGER_FIRED_BEFORE(trigData-tg_event)
!TRIGGER_FIRED_FOR_ROW(trigData-tg_event) )
{
elog(ERROR, trig_ignore_duplicate_updates: Can only be executed for
UPDATE, BEFORE and FOR EACH ROW.);
}
trigData = (TriggerData *) fcinfo-context;
oldTuple = trigData-tg_trigtuple;
newTuple = trigData-tg_newtuple;
if (newTuple-t_len == oldTuple-t_len
newTuple-t_data-t_hoff == oldTuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newTuple-t_data) ==
HeapTupleHeaderGetNatts(oldTuple-t_data)
(newTuple-t_data-t_infomask ~HEAP_XACT_MASK)
== (oldTuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp( (char*)(newTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
(char*)(oldTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newTuple-t_len - offsetof(HeapTupleHeaderData, t_bits)
) == 0 )
{
/* return without crating a new tuple */
return PointerGetDatum( NULL );
}
return PointerGetDatum( trigData-tg_newtuple );
}
--
[EMAIL PROTECTED]
[EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17? 29' 34.37N, 78? 30' 59.76E - Hyderabad *
18? 32' 57.25N, 73? 56' 25.42E - Pune
37? 47' 19.72N, 122? 24' 1.69 W - San Francisco
http://gurjeet.frihost.net
Mail sent from my BlackLaptop device
--
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] minimal update
Andrew Dunstan wrote: Right. In fact, I already had that part in fact - see http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html What I was waiting for was the part where it gets put in the catalog, documented, etc. I can probably do that part. Send over what you have and I will work on it. Thanks. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] minimal update
Bruce Momjian wrote:
Andrew Dunstan wrote:
Right. In fact, I already had that part in fact - see
http://people.planetpostgresql.org/andrew/index.php?/archives/22-Minimal-Update-Trigger.html
What I was waiting for was the part where it gets put in the catalog,
documented, etc.
I can probably do that part. Send over what you have and I will work on
it. Thanks.
It's very similar to what Gurjeet posted (but designed to work with
earlier postgres versions)
cheers
andrew
---
|#include postgres.h
#include commands/trigger.h
#include access/htup.h
#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif
/* for pre 8.3 */
#ifndef HeapTupleHeaderGetNatts
#define HeapTupleHeaderGetNatts(th) ((th)-t_natts )
#endif
extern Datum min_update_trigger(PG_FUNCTION_ARGS);
PG_FUNCTION_INFO_V1(min_update_trigger);
Datum
min_update_trigger(PG_FUNCTION_ARGS)
{
TriggerData *trigdata = (TriggerData *) fcinfo-context;
HeapTuple newtuple, oldtuple, rettuple;
/* make sure it's called as a trigger at all */
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, min_update_trigger: not called by trigger manager);
/* and that it's called on update */
if (! TRIGGER_FIRED_BY_UPDATE(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called on update);
/* and that it's called before update */
if (! TRIGGER_FIRED_BEFORE(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called before update);
/* and that it's called for each row */
if (! TRIGGER_FIRED_FOR_ROW(trigdata-tg_event))
elog(ERROR, min_update_trigger: not called for each row);
/* get tuple dat, set default return */
rettuple = newtuple = trigdata-tg_newtuple;
oldtuple = trigdata-tg_trigtuple;
if (newtuple-t_len == oldtuple-t_len
newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newtuple-t_data) ==
HeapTupleHeaderGetNatts(oldtuple-t_data)
(newtuple-t_data-t_infomask ~HEAP_XACT_MASK) ==
(oldtuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp(((char *)newtuple-t_data) +
offsetof(HeapTupleHeaderData, t_bits),
((char *)oldtuple-t_data) +
offsetof(HeapTupleHeaderData, t_bits),
newtuple-t_len - offsetof(HeapTupleHeaderData,
t_bits)) == 0)
rettuple = NULL;
return PointerGetDatum(rettuple);
}|
--
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] minimal update
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED] wrote: I assume don't want a TODO for this? (Suppress UPDATE no changed columns) I am starting to implement this. Do we want to have this trigger function in the server, or in an external module? Best regards, --- Andrew Dunstan wrote: Tom Lane wrote: Michael Glaesemann [EMAIL PROTECTED] writes: What would be the disadvantages of always doing this, i.e., just making this part of the normal update path in the backend? (1) cycles wasted to no purpose in the vast majority of cases. (2) visibly inconsistent behavior for apps that pay attention to ctid/xmin/etc. (3) visibly inconsistent behavior for apps that have AFTER triggers. There's enough other overhead in issuing an update (network, parsing/planning/etc) that a sanely coded application should try to avoid issuing no-op updates anyway. The proposed trigger is just a band-aid IMHO. I think having it as an optional trigger is a reasonable compromise. Right. I never proposed making this the default behaviour, for all these good reasons. The point about making the app try to avoid no-op updates is that this can impose some quite considerable code complexity on the app, especially where the number of updated fields is large. It's fragile and error-prone. A simple switch that can turn a trigger on or off will be nicer. Syntax support for that might be even nicer, but there appears to be some resistance to that, so I can easily settle for the trigger. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com 17° 29' 34.37N, 78° 30' 59.76E - Hyderabad * 18° 32' 57.25N, 73° 56' 25.42E - Pune 37° 47' 19.72N, 122° 24' 1.69 W - San Francisco http://gurjeet.frihost.net Mail sent from my BlackLaptop device
Re: [HACKERS] minimal update
Gurjeet Singh wrote: On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: I assume don't want a TODO for this? (Suppress UPDATE no changed columns) I am starting to implement this. Do we want to have this trigger function in the server, or in an external module? I have the trigger part of this done, in fact. What remains to be done is to add it to the catalog and document it. The intention is to make it a builtin as it will be generally useful. If you want to work on the remaining parts then I will happily ship you the C code for the trigger. cheers andrew -- 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] minimal update
On Tue, Mar 18, 2008 at 7:46 PM, Andrew Dunstan [EMAIL PROTECTED] wrote:
Gurjeet Singh wrote:
On Fri, Mar 7, 2008 at 9:40 PM, Bruce Momjian [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
I assume don't want a TODO for this? (Suppress UPDATE no changed
columns)
I am starting to implement this. Do we want to have this trigger
function in the server, or in an external module?
I have the trigger part of this done, in fact. What remains to be done
is to add it to the catalog and document it. The intention is to make it
a builtin as it will be generally useful. If you want to work on the
remaining parts then I will happily ship you the C code for the trigger.
In fact, I just finished writing the C code and including it in the catalog
(Just tested that it's visible in the catalog). I will test it to see if it
does actually do what we want it to.
I have incorporated all the suggestions above. Would love to see your code
in the meantime.
Here's the C code:
Datum
trig_ignore_duplicate_updates( PG_FUNCTION_ARGS )
{
TriggerData *trigData;
HeapTuple oldTuple;
HeapTuple newTuple;
if (!CALLED_AS_TRIGGER(fcinfo))
elog(ERROR, trig_ignore_duplicate_updates: not called by trigger
manager.);
if( !TRIGGER_FIRED_BY_UPDATE(trigData-tg_event)
!TRIGGER_FIRED_BEFORE(trigData-tg_event)
!TRIGGER_FIRED_FOR_ROW(trigData-tg_event) )
{
elog(ERROR, trig_ignore_duplicate_updates: Can only be executed for
UPDATE, BEFORE and FOR EACH ROW.);
}
trigData = (TriggerData *) fcinfo-context;
oldTuple = trigData-tg_trigtuple;
newTuple = trigData-tg_newtuple;
if (newTuple-t_len == oldTuple-t_len
newTuple-t_data-t_hoff == oldTuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newTuple-t_data) ==
HeapTupleHeaderGetNatts(oldTuple-t_data)
(newTuple-t_data-t_infomask ~HEAP_XACT_MASK)
== (oldTuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp( (char*)(newTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
(char*)(oldTuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newTuple-t_len - offsetof(HeapTupleHeaderData, t_bits)
) == 0 )
{
/* return without crating a new tuple */
return PointerGetDatum( NULL );
}
return PointerGetDatum( trigData-tg_newtuple );
}
--
[EMAIL PROTECTED]
[EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
17° 29' 34.37N, 78° 30' 59.76E - Hyderabad *
18° 32' 57.25N, 73° 56' 25.42E - Pune
37° 47' 19.72N, 122° 24' 1.69 W - San Francisco
http://gurjeet.frihost.net
Mail sent from my BlackLaptop device
Re: [HACKERS] minimal update
I assume don't want a TODO for this? (Suppress UPDATE no changed columns) --- Andrew Dunstan wrote: Tom Lane wrote: Michael Glaesemann [EMAIL PROTECTED] writes: What would be the disadvantages of always doing this, i.e., just making this part of the normal update path in the backend? (1) cycles wasted to no purpose in the vast majority of cases. (2) visibly inconsistent behavior for apps that pay attention to ctid/xmin/etc. (3) visibly inconsistent behavior for apps that have AFTER triggers. There's enough other overhead in issuing an update (network, parsing/planning/etc) that a sanely coded application should try to avoid issuing no-op updates anyway. The proposed trigger is just a band-aid IMHO. I think having it as an optional trigger is a reasonable compromise. Right. I never proposed making this the default behaviour, for all these good reasons. The point about making the app try to avoid no-op updates is that this can impose some quite considerable code complexity on the app, especially where the number of updated fields is large. It's fragile and error-prone. A simple switch that can turn a trigger on or off will be nicer. Syntax support for that might be even nicer, but there appears to be some resistance to that, so I can easily settle for the trigger. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] minimal update trigger
On Tue, Feb 19, 2008 at 09:32:30PM -0500, Andrew Dunstan wrote:
As discussed a little while back, I would like to add a generic
trigger function which will force an update to skip if the new and
old tuples are identical.
This one has lots of use cases. Did the earlier discussion settle on
whether there should be a GUC and/or CREATE DATABASE and/or initdb
option for this?
Cheers,
David.
The guts of this is the following snippet of code:
|rettuple = newtuple = trigdata-tg_newtuple;
oldtuple = trigdata-tg_trigtuple;
if (newtuple-t_len == oldtuple-t_len
newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff
HeapTupleHeaderGetNatts(newtuple-t_data) ==
HeapTupleHeaderGetNatts(oldtuple-t_data)
(newtuple-t_data-t_infomask ~HEAP_XACT_MASK) ==
(oldtuple-t_data-t_infomask ~HEAP_XACT_MASK)
memcmp(((char *)newtuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
((char *)oldtuple-t_data) + offsetof(HeapTupleHeaderData,
t_bits),
newtuple-t_len - offsetof(HeapTupleHeaderData, t_bits))
== 0)
{
rettuple = NULL;
}
return rettuple;
I propose to call the function pg_minimal_update.
Unless there is an objection I will put together a patch + docs for this
shortly. Not quite sure what section of the docs to put it in - maybe a new
subsection of the Functions chapter?
cheers
andrew
|
---(end of broadcast)---
TIP 6: explain analyze is your friend
--
David Fetter [EMAIL PROTECTED] http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: [EMAIL PROTECTED]
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] minimal update trigger
David Fetter wrote: On Tue, Feb 19, 2008 at 09:32:30PM -0500, Andrew Dunstan wrote: As discussed a little while back, I would like to add a generic trigger function which will force an update to skip if the new and old tuples are identical. This one has lots of use cases. Did the earlier discussion settle on whether there should be a GUC and/or CREATE DATABASE and/or initdb option for this? None of the above. All we will be providing is a trigger function. You would create the trigger as with any other trigger: | CREATE TRIGGER _min BEFORE UPDATE ON mytable FOR EACH ROW EXECUTE PROCEDURE pg_minimal_update(); cheers andrew | ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] minimal update
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: Well, you could write the trigger in C and it'd work for any table. I think it could be as simple as a memcmp of the tuples' data areas, since we now require padding bytes to be 0 ... Something like this fragment? newtuple = trigdata-tg_newtuple; oldtuple = trigdata-tg_trigtuple; rettuple = newtuple; if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff memcmp(GETSTRUCT(newtuple),GETSTRUCT(oldtuple), newtuple-t_len - newtuple-t_data-t_hoff) == 0) rettuple = NULL; return PointerGetDatum(rettuple); Close, but I think you also need to take care to compare natts and the null bitmaps (if any). Might be worth comparing OIDs too, though AFAIR there is no mechanism for substituting a different OID during UPDATE. Probably the easiest coding is to memcmp all the way from offsetof(t_bits) to t_len, after comparing natts and the HASNULL and HASOID flags. How does this look? if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) (newtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) memcmp(newtuple-t_data + offsetof(HeapTupleHeaderData, t_bits), oldtuple-t_data + offsetof(HeapTupleHeaderData, t_bits) newtuple-t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0) rettuple = NULL; return PointerGetDatum(rettuple); cheers andrew Also, when did we first require padding bytes to be 0? The 8.3 varvarlena patch is what requires it, but in practice heap_formtuple has always started with a palloc0, so I think it would work a long ways back. regards, tom lane ---(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: [HACKERS] minimal update
Andrew Dunstan [EMAIL PROTECTED] writes: How does this look? if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) (newtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) memcmp(newtuple-t_data + offsetof(HeapTupleHeaderData, t_bits), oldtuple-t_data + offsetof(HeapTupleHeaderData, t_bits) newtuple-t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0) rettuple = NULL; Looks sane. It might be even saner if you compare all of the non-visibility-related infomask bits, viz (newtuple-t_data-t_infomask ~HEAP_XACT_MASK) == (oldtuple-t_data-t_infomask ~HEAP_XACT_MASK) rather than just HASOID and HASNULL. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] minimal update
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: How does this look? if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) (newtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) memcmp(newtuple-t_data + offsetof(HeapTupleHeaderData, t_bits), oldtuple-t_data + offsetof(HeapTupleHeaderData, t_bits) newtuple-t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0) rettuple = NULL; Looks sane. It might be even saner if you compare all of the non-visibility-related infomask bits, viz (newtuple-t_data-t_infomask ~HEAP_XACT_MASK) == (oldtuple-t_data-t_infomask ~HEAP_XACT_MASK) rather than just HASOID and HASNULL. Sadly, the memcmp is failing on my test (update foo set bar = bar) on 8.2. Looks like I'm in for weekend with my fave debugger :-( cheers andrew ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] minimal update
Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: How does this look? if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff HeapTupleHeaderGetNatts(newtuple) == HeapTupleHeaderGetNatts(oldtuple) (newtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) == (oldtuple-t_data-t_infomask (HEAP_HASOID|HEAP_HASNULL)) memcmp(newtuple-t_data + offsetof(HeapTupleHeaderData, t_bits), oldtuple-t_data + offsetof(HeapTupleHeaderData, t_bits) newtuple-t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0) rettuple = NULL; Looks sane. It might be even saner if you compare all of the non-visibility-related infomask bits, viz (newtuple-t_data-t_infomask ~HEAP_XACT_MASK) == (oldtuple-t_data-t_infomask ~HEAP_XACT_MASK) rather than just HASOID and HASNULL. Sadly, the memcmp is failing on my test (update foo set bar = bar) on 8.2. Looks like I'm in for weekend with my fave debugger :-( Turns out we needed those pointers used in the arguments to memcmp cast to char * so the pointer arithmetic would work right. I'll be suggesting we add a utility function like this for 8.4. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] minimal update
On Nov 2, 2007, at 10:49 AM, Andrew Dunstan wrote: update tname set foo = bar ... where foo is null or foo bar ... FYI, you should be able to do WHERE foo IS DISTINCT FROM bar instead. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] minimal update
Decibel! wrote: On Nov 2, 2007, at 10:49 AM, Andrew Dunstan wrote: update tname set foo = bar ... where foo is null or foo bar ... FYI, you should be able to do WHERE foo IS DISTINCT FROM bar instead. True, that's a bit nicer. It's still more than somewhat ugly and fragile if there a lot of foos and the bars are complex expressions. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] minimal update
On Nov 8, 2007, at 10:46 , Andrew Dunstan wrote: Tom Lane wrote: Michael Glaesemann [EMAIL PROTECTED] writes: What would be the disadvantages of always doing this, i.e., just making this part of the normal update path in the backend? (1) cycles wasted to no purpose in the vast majority of cases. (2) visibly inconsistent behavior for apps that pay attention to ctid/xmin/etc. (3) visibly inconsistent behavior for apps that have AFTER triggers. There's enough other overhead in issuing an update (network, parsing/planning/etc) that a sanely coded application should try to avoid issuing no-op updates anyway. The proposed trigger is just a band-aid IMHO. I think having it as an optional trigger is a reasonable compromise. Right. I never proposed making this the default behaviour, for all these good reasons. The point about making the app try to avoid no-op updates is that this can impose some quite considerable code complexity on the app, especially where the number of updated fields is large. It's fragile and error-prone. A simple switch that can turn a trigger on or off will be nicer. Syntax support for that might be even nicer, but there appears to be some resistance to that, so I can easily settle for the trigger. This confirms what I thought. Thanks. Michael Glaesemann grzm seespotcode net ---(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: [HACKERS] minimal update
On Nov 2, 2007, at 13:44 , Andrew Dunstan wrote: Ah. Good. Thanks, that's the piece I was missing. What would be the disadvantages of always doing this, i.e., just making this part of the normal update path in the backend? I'd think it should save on unnecessarily dead tuples as well. Michael Glaesemann grzm seespotcode net ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] minimal update
Michael Glaesemann [EMAIL PROTECTED] writes: What would be the disadvantages of always doing this, i.e., just making this part of the normal update path in the backend? (1) cycles wasted to no purpose in the vast majority of cases. (2) visibly inconsistent behavior for apps that pay attention to ctid/xmin/etc. (3) visibly inconsistent behavior for apps that have AFTER triggers. There's enough other overhead in issuing an update (network, parsing/planning/etc) that a sanely coded application should try to avoid issuing no-op updates anyway. The proposed trigger is just a band-aid IMHO. I think having it as an optional trigger is a reasonable compromise. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] minimal update
Tom Lane wrote: Michael Glaesemann [EMAIL PROTECTED] writes: What would be the disadvantages of always doing this, i.e., just making this part of the normal update path in the backend? (1) cycles wasted to no purpose in the vast majority of cases. (2) visibly inconsistent behavior for apps that pay attention to ctid/xmin/etc. (3) visibly inconsistent behavior for apps that have AFTER triggers. There's enough other overhead in issuing an update (network, parsing/planning/etc) that a sanely coded application should try to avoid issuing no-op updates anyway. The proposed trigger is just a band-aid IMHO. I think having it as an optional trigger is a reasonable compromise. Right. I never proposed making this the default behaviour, for all these good reasons. The point about making the app try to avoid no-op updates is that this can impose some quite considerable code complexity on the app, especially where the number of updated fields is large. It's fragile and error-prone. A simple switch that can turn a trigger on or off will be nicer. Syntax support for that might be even nicer, but there appears to be some resistance to that, so I can easily settle for the trigger. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] minimal update
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: A BEFORE UPDATE trigger would be better, and probably hardly more expensive than a wired-in facility (especially if you were willing to write it in C). Yes. I also prefer the trigger idea to a rule because triggers are easy to enable and disable. It's still a lot of work for what must be a common want, though. Could it be done generically? Well, you could write the trigger in C and it'd work for any table. I think it could be as simple as a memcmp of the tuples' data areas, since we now require padding bytes to be 0 ... Something like this fragment? newtuple = trigdata-tg_newtuple; oldtuple = trigdata-tg_trigtuple; rettuple = newtuple; if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff memcmp(GETSTRUCT(newtuple),GETSTRUCT(oldtuple), newtuple-t_len - newtuple-t_data-t_hoff) == 0) rettuple = NULL; return PointerGetDatum(rettuple); Also, when did we first require padding bytes to be 0? cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] minimal update
Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: Well, you could write the trigger in C and it'd work for any table. I think it could be as simple as a memcmp of the tuples' data areas, since we now require padding bytes to be 0 ... Something like this fragment? newtuple = trigdata-tg_newtuple; oldtuple = trigdata-tg_trigtuple; rettuple = newtuple; if (newtuple-t_len == oldtuple-t_len newtuple-t_data-t_hoff == oldtuple-t_data-t_hoff memcmp(GETSTRUCT(newtuple),GETSTRUCT(oldtuple), newtuple-t_len - newtuple-t_data-t_hoff) == 0) rettuple = NULL; return PointerGetDatum(rettuple); Close, but I think you also need to take care to compare natts and the null bitmaps (if any). Might be worth comparing OIDs too, though AFAIR there is no mechanism for substituting a different OID during UPDATE. Probably the easiest coding is to memcmp all the way from offsetof(t_bits) to t_len, after comparing natts and the HASNULL and HASOID flags. Also, when did we first require padding bytes to be 0? The 8.3 varvarlena patch is what requires it, but in practice heap_formtuple has always started with a palloc0, so I think it would work a long ways back. regards, tom lane ---(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: [HACKERS] minimal update
David Fetter [EMAIL PROTECTED] writes: On Fri, Nov 02, 2007 at 11:49:38AM -0400, Andrew Dunstan wrote: At the moment I have to write things like: update tname set foo = bar ... where foo is null or foo bar One way I've done this is make RULEs which basically drop non-updating UPDATEs on the floor. A BEFORE UPDATE trigger would be better, and probably hardly more expensive than a wired-in facility (especially if you were willing to write it in C). regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] minimal update
Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Fri, Nov 02, 2007 at 11:49:38AM -0400, Andrew Dunstan wrote: At the moment I have to write things like: update tname set foo = bar ... where foo is null or foo bar One way I've done this is make RULEs which basically drop non-updating UPDATEs on the floor. A BEFORE UPDATE trigger would be better, and probably hardly more expensive than a wired-in facility (especially if you were willing to write it in C). Yes. I also prefer the trigger idea to a rule because triggers are easy to enable and disable. It's still a lot of work for what must be a common want, though. Could it be done generically? cheers andrew ---(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: [HACKERS] minimal update
On Fri, Nov 02, 2007 at 11:49:38AM -0400, Andrew Dunstan wrote: For some time I have been working on removing some inefficiencies from a large DW-type app. This app does a large daily batch update, and this is what is the major bottleneck. One of the things I have been doing is to remove unnecessary updates (which are particualrly expensive in our index-rich setting). Several times now I have wished that there was a switch on the UPDATE command that said do minimal instead of maximal updating. i.e., don't update records with identical replacements. At the moment I have to write things like: update tname set foo = bar ... where foo is null or foo bar ... One way I've done this is make RULEs which basically drop non-updating UPDATEs on the floor. CREATE RULE foo_drop_empty_updates AS ON UPDATE TO foo WHERE ROW(OLD.*)::foo IS NOT DISTINCT FROM ROW(NEW.*)::foo DO INSTEAD NOTHING; It's pretty easy to automate rule creation, but since Postgres doesn't have DDL triggers, it's also a bit of a foot gun. By the way, the above has what I think of as an infelicity in 8.2.5, namely that you need non-obvious contortions to get it to work. I'm thinking OLD IS NOT DISTINCT FROM NEW should Just Work(TM). This becomes more than tedious when the update might be setting thirty or forty fields, and I have to write such tests for each of them. It would be so much nicer to be able to write something like: update tname minimally set foo = bar ... Is this an insane idea, or would it be possible, practical and useful? I don't know about the sanity, but I've done it a couple of places :) Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate ---(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: [HACKERS] minimal update
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: A BEFORE UPDATE trigger would be better, and probably hardly more expensive than a wired-in facility (especially if you were willing to write it in C). Yes. I also prefer the trigger idea to a rule because triggers are easy to enable and disable. It's still a lot of work for what must be a common want, though. Could it be done generically? Well, you could write the trigger in C and it'd work for any table. I think it could be as simple as a memcmp of the tuples' data areas, since we now require padding bytes to be 0 ... Ah. Good. Thanks, that's the piece I was missing. cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
