clarifying trigger/rule behavior on logical replication subscribers

On Wed, 7 Jun 2023 at 10:17, Jonathan S. Katz <jkatz@postgresql.org> wrote:

Hi,

While answering a question on "do triggers fire on a logical replication
subscriber by default?" I tried to look up a reference to this behavior
in the docs. There wasn't a clear reference point, but on the
architecture page[1], I found this line that was closest to the answer:

"The apply process on the subscriber database always runs with
session_replication_role set to replica, which produces the usual
effects on triggers and constraints."

which assumes that the reader knows what the "usual effects" are.

Attached is a patch that disambiguates this.

Thanks for the patch, one small change required, "literal>" should be
"<literal>":
+   enable triggers and rules on a table using the
+   <link linkend="sql-altertable"><command>ALTER TABLE</command></link> command
+   and the literal>ENABLE TRIGGER</literal> and <literal>ENABLE RULE</literal>

Regards,
Vignesh

On Wed, Jun 7, 2023 at 4:09 PM vignesh C <vignesh21@gmail.com> wrote:

On Wed, 7 Jun 2023 at 10:17, Jonathan S. Katz <jkatz@postgresql.org> wrote:

Hi,

While answering a question on "do triggers fire on a logical replication
subscriber by default?" I tried to look up a reference to this behavior
in the docs. There wasn't a clear reference point, but on the
architecture page[1], I found this line that was closest to the answer:

"The apply process on the subscriber database always runs with
session_replication_role set to replica, which produces the usual
effects on triggers and constraints."

which assumes that the reader knows what the "usual effects" are.

Attached is a patch that disambiguates this.

I agree that it is a good idea to be explicit about this behavior. As
mentioned by you, it would be better to backpatch this. Also, I
thought about moving this to some other section like [1]https://www.postgresql.org/docs/devel/logical-replication-subscription.html but I think
we need to explain a bit about apply worker before providing this
specific information. So, the current place seems to be the best bet
considering the way current logical replication is documented.

[1]: https://www.postgresql.org/docs/devel/logical-replication-subscription.html

--
With Regards,
Amit Kapila.

On Thu, Jun 8, 2023, at 3:36 AM, Amit Kapila wrote:

I agree that it is a good idea to be explicit about this behavior. As
mentioned by you, it would be better to backpatch this. Also, I
thought about moving this to some other section like [1] but I think
we need to explain a bit about apply worker before providing this
specific information. So, the current place seems to be the best bet
considering the way current logical replication is documented.

I agree with this clarification in the suggested section. One small suggestion
is to avoid the word "replica", AFAICS it is not used in the logical
replication chapter; use "subscriber" instead. Another suggestion is to add
links to the exact ALTER TABLE ... ENABLE TRIGGER and ALTER TABLE ... ENABLE
RULE since we recently added it.

--
Euler Taveira
EDB https://www.enterprisedb.com/

Hi.

I have made a patch v2.

This addresses the following:
- [1]/messages/by-id/CALDaNm1fAaeOhdTts3LgXs_e1TPZke3-HqFA9b8ZjVXOM66H5Q@mail.gmail.com Vignesh's review comment about unclosed SGML literal tag.
- [2]/messages/by-id/288747bd-911b-4370-8db2-6ecbb096037e@app.fastmail.com Euler's review comment about the word "replica"

But I did not address Euler's other review suggestion [2]/messages/by-id/288747bd-911b-4370-8db2-6ecbb096037e@app.fastmail.com about giving
direct links, because actually this patch text (without any direct
links) is almost identical with the existing
"session_replication_role" docs [3]https://www.postgresql.org/docs/devel/runtime-config-client.html. Instead of making the scope of
this patch bigger, if direct links are really wanted, I thought it is
better to have a separate patch for that which can address both docs.

~~~

I also tested/attached back-patches of the v2 patch for earlier
PostgreSQL versions. Actually, it turns out all the patches are the
same -- just named differently.

------
[1]: /messages/by-id/CALDaNm1fAaeOhdTts3LgXs_e1TPZke3-HqFA9b8ZjVXOM66H5Q@mail.gmail.com
[2]: /messages/by-id/288747bd-911b-4370-8db2-6ecbb096037e@app.fastmail.com
[3]: https://www.postgresql.org/docs/devel/runtime-config-client.html

Kind Regards,
Peter Smith.
Fujitsu Australia

I created patches for this over a month ago, but then the thread went
quiet. Is there something more needed, or was it just accidentally
overlooked?

Should this be added to the commitfest?

------
Kind Regards,
Peter Smith.
Fujitsu Australia.

On Thu, 2023-07-27 at 12:16 +1000, Peter Smith wrote:

I created patches for this over a month ago, but then the thread went
quiet. Is there something more needed, or was it just accidentally
overlooked?

Should this be added to the commitfest?

Yes, please add it to the commitfest. That increases the likelyhood of
you patch not being forgotten.

Yours,
Laurenz Albe

It seems this was pushed already [1]https://github.com/postgres/postgres/commit/f5c446e3367527f9db1506d7c38d2f56e20950b6. Sorry for the false alarm.

------
[1]: https://github.com/postgres/postgres/commit/f5c446e3367527f9db1506d7c38d2f56e20950b6

Kind Regards,
Peter Smith.
Fujitsu Australia.