Not using suppress_redundant_updates_trigger in sql-createtrigger.html#examples

On Fri, Jun 12, 2020 at 09:09:09AM +0000, PG Doc comments form wrote:

The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/12/bug-reporting.html
Description:

Hi
I was stumbling across the trigger function
‘suppress_redundant_updates_trigger’ in
https://www.postgresql.org/docs/devel/functions-trigger.html and I would
just ask if there is any reason of this not being int the documentation in
https://www.postgresql.org/docs/13/sql-createtrigger.html.
So after

Call a function to log updates of accounts, but only if something changed:

CREATE TRIGGER log_update
AFTER UPDATE ON accounts
FOR EACH ROW
WHEN (OLD.* IS DISTINCT FROM NEW.*)
EXECUTE FUNCTION log_account_update();

There could be an alternative implementation as well

CREATE TRIGGER log_update
AFTER UPDATE ON accounts
FOR EACH ROW
EXECUTE FUNCTION log_account_update();

CREATE TRIGGER suppress_redundant_account_after_updates
AFTER UPDATE ON accounts
FOR EACH ROW EXECUTE FUNCTION suppress_redundant_updates_trigger();

Just to introduce and like the feature.

Thank you for your great work best regards

I have created the attached patch to mention
suppress_redundant_updates_trigger() in this case. I don't think having
an actual example is warranted.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Bruce Momjian <bruce@momjian.us> writes:

I have created the attached patch to mention
suppress_redundant_updates_trigger() in this case. I don't think having
an actual example is warranted.

I don't like this patch, because in fact
suppress_redundant_updates_trigger is entirely unrelated to the stated
purpose of that example (namely, to log something if the table was
changed). The way you've written it makes it sound like
suppress_redundant_updates_trigger could be used as an alternative
implementation of that requirement.

If you want to mention suppress_redundant_updates_trigger somewhere
in this area, that's fine, but it should be treated as an independent
topic rather than being wedged into the middle of an unrelated example.
Maybe a distinct para saying something like "To suppress no-op updates
of a table, see suppress_redundant_updates_trigger." (and making that
an actual link would be a good idea).

regards, tom lane

On Tue, Jun 16, 2020 at 10:08:14AM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I have created the attached patch to mention
suppress_redundant_updates_trigger() in this case. I don't think having
an actual example is warranted.

I don't like this patch, because in fact
suppress_redundant_updates_trigger is entirely unrelated to the stated
purpose of that example (namely, to log something if the table was
changed). The way you've written it makes it sound like
suppress_redundant_updates_trigger could be used as an alternative
implementation of that requirement.

Ah, I see your point.

If you want to mention suppress_redundant_updates_trigger somewhere
in this area, that's fine, but it should be treated as an independent
topic rather than being wedged into the middle of an unrelated example.
Maybe a distinct para saying something like "To suppress no-op updates
of a table, see suppress_redundant_updates_trigger." (and making that
an actual link would be a good idea).

I have developed the attached patch which is in a better direction.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Bruce Momjian <bruce@momjian.us> writes:

I have developed the attached patch which is in a better direction.

I still wish you wouldn't jam this topic into an only-marginally-related
sentence. In the case at hand, the point that the existing text is
making is that a no-op update will fire triggers. Which is not
something that suppress_redundant_updates_trigger will suppress,
so again it seems confusing to mention that in the same breath.

I think that perhaps the right thing to do is add a new para to
CREATE TRIGGER's "Notes" section, along the lines of

There are a few built-in trigger functions that can be used
to solve common problems without having to write your own trigger
code. See Section 9.28.

This would have the advantage of covering the other built-in triggers
not only suppress_redundant_updates_trigger.

regards, tom lane

On Tue, Jun 16, 2020 at 01:39:45PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I have developed the attached patch which is in a better direction.

I still wish you wouldn't jam this topic into an only-marginally-related
sentence. In the case at hand, the point that the existing text is
making is that a no-op update will fire triggers. Which is not
something that suppress_redundant_updates_trigger will suppress,
so again it seems confusing to mention that in the same breath.

I think that perhaps the right thing to do is add a new para to
CREATE TRIGGER's "Notes" section, along the lines of

There are a few built-in trigger functions that can be used
to solve common problems without having to write your own trigger
code. See Section 9.28.

This would have the advantage of covering the other built-in triggers
not only suppress_redundant_updates_trigger.

OK, I didn't think there was enough desire to put it its own paragraph,
but I like the idea of mentioning all of the trigger functions; patch
attached.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Bruce Momjian <bruce@momjian.us> writes:

OK, I didn't think there was enough desire to put it its own paragraph,
but I like the idea of mentioning all of the trigger functions; patch
attached.

This one WFM.

regards, tom lane

On Tue, Jun 16, 2020 at 02:02:20PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

OK, I didn't think there was enough desire to put it its own paragraph,
but I like the idea of mentioning all of the trigger functions; patch
attached.

This one WFM.

Patch applied to all supported versions.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee