Horton

Thread

Commits

  1. postgres_fdw: Inherit the local transaction's access/deferrable modes.

  2. Revert "postgres_fdw: Inherit the local transaction's access/deferrable modes."

  1. pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <efujita@postgresql.org> — 2025-06-01T08:34:49Z

    postgres_fdw: Inherit the local transaction's access/deferrable modes.

Previously, postgres_fdw always 1) opened a remote transaction in READ
WRITE mode even when the local transaction was READ ONLY, causing a READ
ONLY transaction using it that references a foreign table mapped to a
remote view executing a volatile function to write in the remote side,
and 2) opened the remote transaction in NOT DEFERRABLE mode even when
the local transaction was DEFERRABLE, causing a SERIALIZABLE READ ONLY
DEFERRABLE transaction using it to abort due to a serialization failure
in the remote side.

To avoid these, modify postgres_fdw to open a remote transaction in the
same access/deferrable modes as the local transaction.  This commit also
modifies it to open a remote subtransaction in the same access mode as
the local subtransaction.

Although these issues exist since the introduction of postgres_fdw,
there have been no reports from the field.  So it seems fine to just fix
them in master only.

Author: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAPmGK16n_hcUUWuOdmeUS%2Bw4Q6dZvTEDHb%3DOP%3D5JBzo-M3QmpQ%40mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/e5a3c9d9b5ce535151d3a7e3173e8d27d2d8cd58

Modified Files
--------------
contrib/postgres_fdw/connection.c              |  99 ++++++++++++++++--
contrib/postgres_fdw/expected/postgres_fdw.out | 134 +++++++++++++++++++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql      |  78 ++++++++++++++
doc/src/sgml/postgres-fdw.sgml                 |  15 +++
src/backend/access/transam/xact.c              |  28 ++++++
src/include/access/xact.h                      |   1 +
6 files changed, 347 insertions(+), 8 deletions(-)

  2. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-06-02T03:03:50Z

    
On 2025/06/01 17:34, Etsuro Fujita wrote:
> postgres_fdw: Inherit the local transaction's access/deferrable modes.
> 
> Previously, postgres_fdw always 1) opened a remote transaction in READ
> WRITE mode even when the local transaction was READ ONLY, causing a READ
> ONLY transaction using it that references a foreign table mapped to a
> remote view executing a volatile function to write in the remote side,
> and 2) opened the remote transaction in NOT DEFERRABLE mode even when
> the local transaction was DEFERRABLE, causing a SERIALIZABLE READ ONLY
> DEFERRABLE transaction using it to abort due to a serialization failure
> in the remote side.
> 
> To avoid these, modify postgres_fdw to open a remote transaction in the
> same access/deferrable modes as the local transaction.  This commit also
> modifies it to open a remote subtransaction in the same access mode as
> the local subtransaction.
> 
> Although these issues exist since the introduction of postgres_fdw,
> there have been no reports from the field.  So it seems fine to just fix
> them in master only.

I'm not sure this change should be considered a bug fix,
since the current behavior of postgres_fdw with a local read-only
transaction isn't clearly documented. Some users might see this
as a behavioral change rather than a fix. Anyway if we go with it,
shouldn't we document the change in the v18 release notes?

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  3. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Michael Paquier <michael@paquier.xyz> — 2025-06-02T03:33:23Z

    On Mon, Jun 02, 2025 at 12:03:50PM +0900, Fujii Masao wrote:
> I'm not sure this change should be considered a bug fix,
> since the current behavior of postgres_fdw with a local read-only
> transaction isn't clearly documented. Some users might see this
> as a behavioral change rather than a fix. Anyway if we go with it,
> shouldn't we document the change in the v18 release notes?

After going through the thread and the commit, I have to admit that I
was surprised to see this applied on HEAD now that we are in feature
freeze.  This is a behavior change.  Perhaps this could be done once
v19 happens, still it's rather unclear if the new behavior is better
than the previous one.
--
Michael

  4. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-06-03T10:45:34Z

    On Mon, Jun 2, 2025 at 12:33 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Jun 02, 2025 at 12:03:50PM +0900, Fujii Masao wrote:
> > I'm not sure this change should be considered a bug fix,
> > since the current behavior of postgres_fdw with a local read-only
> > transaction isn't clearly documented. Some users might see this
> > as a behavioral change rather than a fix. Anyway if we go with it,
> > shouldn't we document the change in the v18 release notes?
>
> After going through the thread and the commit, I have to admit that I
> was surprised to see this applied on HEAD now that we are in feature
> freeze.  This is a behavior change.  Perhaps this could be done once
> v19 happens, still it's rather unclear if the new behavior is better
> than the previous one.

No, this is a fix, not a feature, as discussed in the thread; as
mentioned in the commit message, the previous version of postgres_fdw
could cause surprising behaviors that would never happen in normal
cases where a read-only and/or deferrable transaction only
accesses/modifies data on the local server, so this commit fixes those
behaviors.  But yes, it makes a behavior change, so I think it’s a
good idea to add a note about that to the v18 release notes, as
proposed by Fujii-san.

Thank you for the comments!

Best regards,
Etsuro Fujita

  5. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-06-04T04:14:44Z

    
On 2025/06/03 19:45, Etsuro Fujita wrote:
> On Mon, Jun 2, 2025 at 12:33 PM Michael Paquier <michael@paquier.xyz> wrote:
>> On Mon, Jun 02, 2025 at 12:03:50PM +0900, Fujii Masao wrote:
>>> I'm not sure this change should be considered a bug fix,
>>> since the current behavior of postgres_fdw with a local read-only
>>> transaction isn't clearly documented. Some users might see this
>>> as a behavioral change rather than a fix. Anyway if we go with it,
>>> shouldn't we document the change in the v18 release notes?
>>
>> After going through the thread and the commit, I have to admit that I
>> was surprised to see this applied on HEAD now that we are in feature
>> freeze.  This is a behavior change.  Perhaps this could be done once
>> v19 happens, still it's rather unclear if the new behavior is better
>> than the previous one.
> 
> No, this is a fix, not a feature, as discussed in the thread; as
> mentioned in the commit message, the previous version of postgres_fdw
> could cause surprising behaviors that would never happen in normal
> cases where a read-only and/or deferrable transaction only
> accesses/modifies data on the local server, so this commit fixes those
> behaviors.

I agree this could be considered a fix if the new behavior has been
clearly explained in the documentation from before or based on
standards like SQL/MED. But if that's not the case, it seems more
like a behavior change. In that case, I think it should wait for v19
and be applied only after reaching consensus. Some systems might
rely on the previous behavior.

By the way, if a read-only transaction on the local server is meant
to block all write operations on the remote server, this patch alone
might not be sufficient, for example, that read-only transaction can
invoke a login trigger on the remote server and it could still
perform writes.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  6. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Robert Haas <robertmhaas@gmail.com> — 2025-06-04T18:39:17Z

    On Tue, Jun 3, 2025 at 6:45 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> No, this is a fix, not a feature, as discussed in the thread; as
> mentioned in the commit message, the previous version of postgres_fdw
> could cause surprising behaviors that would never happen in normal
> cases where a read-only and/or deferrable transaction only
> accesses/modifies data on the local server, so this commit fixes those
> behaviors.  But yes, it makes a behavior change, so I think it’s a
> good idea to add a note about that to the v18 release notes, as
> proposed by Fujii-san.

Sometimes, people can have different opinions about whether something
is a bug fix or a behavior change. So far, I don't think you've
convinced a single person either on the original thread or on this one
that this is a bug fix, so I believe that, at present, the consensus
is that this is a new feature. Although you may not agree with that
consensus, and you may even be right, we all have to do what most
people agree is right rather than what we ourselves prefer.

For what it's worth, I agree with others that this is not just a bug
fix: it's a behavior change that should be subject to the feature
freeze. I personally think that it's probably a desirable behavior
change, and that it's small enough that we could consider leaving it
in v18 if that meets with general approval. We have had cases like
this, where something feels too disruptive to back-patch, but is still
on some level a fix or correction of behavior, in the past, and we
have sometimes decided to handle those by allowing them to be added to
the major release after the feature freeze deadline, but not
back-patching them. So in my mind that is a possibility here.

However, that would require a pretty unanimous agreement that this
change is an improvement, and it appears to me that we don't have
that. I read Fujii Masao's comments to indicate that he doesn't
necessarily agree with the change and wants it reverted, and I read
Michael Paquier's comments the same way. Unless I'm misunderstanding
their position, this needs to be reverted.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  7. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-06-05T10:40:43Z

    On Thu, Jun 5, 2025 at 3:39 AM Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Jun 3, 2025 at 6:45 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > No, this is a fix, not a feature, as discussed in the thread; as
> > mentioned in the commit message, the previous version of postgres_fdw
> > could cause surprising behaviors that would never happen in normal
> > cases where a read-only and/or deferrable transaction only
> > accesses/modifies data on the local server, so this commit fixes those
> > behaviors.  But yes, it makes a behavior change, so I think it’s a
> > good idea to add a note about that to the v18 release notes, as
> > proposed by Fujii-san.
>
> Sometimes, people can have different opinions about whether something
> is a bug fix or a behavior change. So far, I don't think you've
> convinced a single person either on the original thread or on this one
> that this is a bug fix, so I believe that, at present, the consensus
> is that this is a new feature. Although you may not agree with that
> consensus, and you may even be right, we all have to do what most
> people agree is right rather than what we ourselves prefer.

A consensus we reached on the original thread is that if the previous
behavior is considered problematic, we should fix it; otherwise, we
should not.  I proposed to fix it for the reason mentioned above, and
went ahead, as there were no objections about that.  But seeing the
comments on this thread, I have to agree that this is a feature rather
than a fix.

> For what it's worth, I agree with others that this is not just a bug
> fix: it's a behavior change that should be subject to the feature
> freeze. I personally think that it's probably a desirable behavior
> change, and that it's small enough that we could consider leaving it
> in v18 if that meets with general approval. We have had cases like
> this, where something feels too disruptive to back-patch, but is still
> on some level a fix or correction of behavior, in the past, and we
> have sometimes decided to handle those by allowing them to be added to
> the major release after the feature freeze deadline, but not
> back-patching them. So in my mind that is a possibility here.
>
> However, that would require a pretty unanimous agreement that this
> change is an improvement, and it appears to me that we don't have
> that. I read Fujii Masao's comments to indicate that he doesn't
> necessarily agree with the change and wants it reverted, and I read
> Michael Paquier's comments the same way. Unless I'm misunderstanding
> their position, this needs to be reverted.

Agreed.  I will revert this in a few days.  And I will re-propose it
as an improvement for v19.

Thanks for the discussion!

Best regards,
Etsuro Fujita

  8. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2025-06-08T08:45:23Z

    On Thu, Jun 5, 2025 at 7:40 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> I will revert this in a few days.

Done.

Best regards,
Etsuro Fujita

  9. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2026-02-15T08:40:13Z

    On Wed, Jun 4, 2025 at 1:15 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> I agree this could be considered a fix if the new behavior has been
> clearly explained in the documentation from before or based on
> standards like SQL/MED. But if that's not the case, it seems more
> like a behavior change. In that case, I think it should wait for v19
> and be applied only after reaching consensus. Some systems might
> rely on the previous behavior.
>
> By the way, if a read-only transaction on the local server is meant
> to block all write operations on the remote server, this patch alone
> might not be sufficient, for example, that read-only transaction can
> invoke a login trigger on the remote server and it could still
> perform writes.

This patch 1) modifies postgres_fdw so that it opens remote
transactions in read-only mode if the corresponding local transaction
is read-only, as noted in the documentation, but 2) keeps the existing
behavior of login triggers that they can write even if the invoking
transaction is read-only.  So declaring a transaction as read-only on
the local side doesn't mean it blocks all write operations on the
remote side; it still allows login triggers invoked on the remote side
to write.  Considering typical use-cases of such triggers, this seems
reasonable to me.  I think it might be a good idea to add a note about
it to the documentation, though.

I'd like to re-propose this patch for v19, as mentioned in this thread.

Thanks!

Best regards,
Etsuro Fujita

  10. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Michael Paquier <michael@paquier.xyz> — 2026-02-16T01:15:08Z

    On Sun, Feb 15, 2026 at 05:40:13PM +0900, Etsuro Fujita wrote:
> This patch 1) modifies postgres_fdw so that it opens remote
> transactions in read-only mode if the corresponding local transaction
> is read-only, as noted in the documentation, but 2) keeps the existing
> behavior of login triggers that they can write even if the invoking
> transaction is read-only.  So declaring a transaction as read-only on
> the local side doesn't mean it blocks all write operations on the
> remote side; it still allows login triggers invoked on the remote side
> to write.  Considering typical use-cases of such triggers, this seems
> reasonable to me.  I think it might be a good idea to add a note about
> it to the documentation, though.
> 
> I'd like to re-propose this patch for v19, as mentioned in this thread.

Considering again that for v19 sounds like a sensible thing to do.
Before feature freeze, not after.  :D
--
Michael

  11. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2026-03-05T00:00:55Z

    On Sun, Feb 15, 2026 at 5:40 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Wed, Jun 4, 2025 at 1:15 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> > I agree this could be considered a fix if the new behavior has been
> > clearly explained in the documentation from before or based on
> > standards like SQL/MED. But if that's not the case, it seems more
> > like a behavior change. In that case, I think it should wait for v19
> > and be applied only after reaching consensus. Some systems might
> > rely on the previous behavior.
> >
> > By the way, if a read-only transaction on the local server is meant
> > to block all write operations on the remote server, this patch alone
> > might not be sufficient, for example, that read-only transaction can
> > invoke a login trigger on the remote server and it could still
> > perform writes.
>
> This patch 1) modifies postgres_fdw so that it opens remote
> transactions in read-only mode if the corresponding local transaction
> is read-only, as noted in the documentation, but 2) keeps the existing
> behavior of login triggers that they can write even if the invoking
> transaction is read-only.  So declaring a transaction as read-only on
> the local side doesn't mean it blocks all write operations on the
> remote side; it still allows login triggers invoked on the remote side
> to write.  Considering typical use-cases of such triggers, this seems
> reasonable to me.  I think it might be a good idea to add a note about
> it to the documentation, though.
>
> I'd like to re-propose this patch for v19, as mentioned in this thread.

I posted a new version of the patch to the -hackers mailing list [1],
which includes the note mentioned above.  It would be great if I got
feedback from you.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/message-id/CAPmGK14ZTHRGPprEhzEe2TJxaCcjNVeWw6tue_gqp%3D9DzqYnMA%40mail.gmail.com

  12. Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2026-03-05T00:04:51Z

    On Mon, Feb 16, 2026 at 10:15 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Sun, Feb 15, 2026 at 05:40:13PM +0900, Etsuro Fujita wrote:
> > This patch 1) modifies postgres_fdw so that it opens remote
> > transactions in read-only mode if the corresponding local transaction
> > is read-only, as noted in the documentation, but 2) keeps the existing
> > behavior of login triggers that they can write even if the invoking
> > transaction is read-only.  So declaring a transaction as read-only on
> > the local side doesn't mean it blocks all write operations on the
> > remote side; it still allows login triggers invoked on the remote side
> > to write.  Considering typical use-cases of such triggers, this seems
> > reasonable to me.  I think it might be a good idea to add a note about
> > it to the documentation, though.
> >
> > I'd like to re-propose this patch for v19, as mentioned in this thread.
>
> Considering again that for v19 sounds like a sensible thing to do.
> Before feature freeze, not after.  :D

Will do.

Best regards,
Etsuro Fujita