public inbox for [email protected]help / color / mirror / Atom feed
pgsql: postgres_fdw: Inherit the local transaction's access/deferrable 12+ messages / 5 participants [nested] [flat]
* pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-01 08:34 Etsuro Fujita <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Etsuro Fujita @ 2025-06-01 08:34 UTC (permalink / raw) To: [email protected] 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 <[email protected]> Reviewed-by: Ashutosh Bapat <[email protected]> Reviewed-by: Tom Lane <[email protected]> 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(-) ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-02 03:03 Fujii Masao <[email protected]> parent: Etsuro Fujita <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Fujii Masao @ 2025-06-02 03:03 UTC (permalink / raw) To: Etsuro Fujita <[email protected]>; [email protected] 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-02 03:33 Michael Paquier <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Michael Paquier @ 2025-06-02 03:33 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Etsuro Fujita <[email protected]>; [email protected] 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 Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-03 10:45 Etsuro Fujita <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 2 replies; 12+ messages in thread From: Etsuro Fujita @ 2025-06-03 10:45 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Fujii Masao <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Mon, Jun 2, 2025 at 12:33 PM Michael Paquier <[email protected]> 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-04 04:14 Fujii Masao <[email protected]> parent: Etsuro Fujita <[email protected]> 1 sibling, 1 reply; 12+ messages in thread From: Fujii Masao @ 2025-06-04 04:14 UTC (permalink / raw) To: Etsuro Fujita <[email protected]>; Michael Paquier <[email protected]>; +Cc: Etsuro Fujita <[email protected]>; [email protected] On 2025/06/03 19:45, Etsuro Fujita wrote: > On Mon, Jun 2, 2025 at 12:33 PM Michael Paquier <[email protected]> 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-04 18:39 Robert Haas <[email protected]> parent: Etsuro Fujita <[email protected]> 1 sibling, 1 reply; 12+ messages in thread From: Robert Haas @ 2025-06-04 18:39 UTC (permalink / raw) To: Etsuro Fujita <[email protected]>; +Cc: Michael Paquier <[email protected]>; Fujii Masao <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Tue, Jun 3, 2025 at 6:45 AM Etsuro Fujita <[email protected]> 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-05 10:40 Etsuro Fujita <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 1 reply; 12+ messages in thread From: Etsuro Fujita @ 2025-06-05 10:40 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Michael Paquier <[email protected]>; Fujii Masao <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Thu, Jun 5, 2025 at 3:39 AM Robert Haas <[email protected]> wrote: > On Tue, Jun 3, 2025 at 6:45 AM Etsuro Fujita <[email protected]> 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2025-06-08 08:45 Etsuro Fujita <[email protected]> parent: Etsuro Fujita <[email protected]> 0 siblings, 0 replies; 12+ messages in thread From: Etsuro Fujita @ 2025-06-08 08:45 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Michael Paquier <[email protected]>; Fujii Masao <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Thu, Jun 5, 2025 at 7:40 PM Etsuro Fujita <[email protected]> wrote: > I will revert this in a few days. Done. Best regards, Etsuro Fujita ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2026-02-15 08:40 Etsuro Fujita <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 2 replies; 12+ messages in thread From: Etsuro Fujita @ 2026-02-15 08:40 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Michael Paquier <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Wed, Jun 4, 2025 at 1:15 PM Fujii Masao <[email protected]> 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2026-02-16 01:15 Michael Paquier <[email protected]> parent: Etsuro Fujita <[email protected]> 1 sibling, 1 reply; 12+ messages in thread From: Michael Paquier @ 2026-02-16 01:15 UTC (permalink / raw) To: Etsuro Fujita <[email protected]>; +Cc: Fujii Masao <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] 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 Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2026-03-05 00:00 Etsuro Fujita <[email protected]> parent: Etsuro Fujita <[email protected]> 1 sibling, 0 replies; 12+ messages in thread From: Etsuro Fujita @ 2026-03-05 00:00 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Michael Paquier <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Sun, Feb 15, 2026 at 5:40 PM Etsuro Fujita <[email protected]> wrote: > On Wed, Jun 4, 2025 at 1:15 PM Fujii Masao <[email protected]> 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.g... ^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: postgres_fdw: Inherit the local transaction's access/deferrable @ 2026-03-05 00:04 Etsuro Fujita <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 12+ messages in thread From: Etsuro Fujita @ 2026-03-05 00:04 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Fujii Masao <[email protected]>; Etsuro Fujita <[email protected]>; [email protected] On Mon, Feb 16, 2026 at 10:15 AM Michael Paquier <[email protected]> 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 ^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2026-03-05 00:04 UTC | newest] Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-06-01 08:34 pgsql: postgres_fdw: Inherit the local transaction's access/deferrable Etsuro Fujita <[email protected]> 2025-06-02 03:03 ` Fujii Masao <[email protected]> 2025-06-02 03:33 ` Michael Paquier <[email protected]> 2025-06-03 10:45 ` Etsuro Fujita <[email protected]> 2025-06-04 04:14 ` Fujii Masao <[email protected]> 2026-02-15 08:40 ` Etsuro Fujita <[email protected]> 2026-02-16 01:15 ` Michael Paquier <[email protected]> 2026-03-05 00:04 ` Etsuro Fujita <[email protected]> 2026-03-05 00:00 ` Etsuro Fujita <[email protected]> 2025-06-04 18:39 ` Robert Haas <[email protected]> 2025-06-05 10:40 ` Etsuro Fujita <[email protected]> 2025-06-08 08:45 ` Etsuro Fujita <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox