Re: Options to control remote transactions’ access/deferrable modes in postgres_fdw

On Mon, Mar 3, 2025 at 4:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
> > On Sun, Mar 2, 2025 at 5:14 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> >> To avoid that, I would like to propose a server option,
> >> inherit_read_only, to open the remote transactions in read-only mode
> >> if the local transaction is read-only.
>
> > Why do we need a server option. Either we say that a local READ ONLY
> > transaction causing modifications on the foreign server is problematic
> > or it's expected. But what's the point in giving that choice to the
> > user? If we deem the behaviour problematic it should be considered as
> > a bug and we should fix it. Otherwise not fix it.
>
> I tend to agree with Ashutosh's position here.  Reasoning about
> issues like this is hard enough already.  Having to figure out an
> application's behavior under more than one setting makes it harder.
> You may argue that "then the application can choose the behavior it
> likes, so there's no need to figure out both behaviors".  But for a
> lot of bits of code, that's not the situation; rather, they have to
> be prepared to work under both settings, because someone else is
> in charge of what the setting is.  (I don't know if either of you
> recall our disastrous attempt at server-side autocommit, back around
> 7.3.  The reason that got reverted was exactly that there was too
> much code that had to be prepared to work under either setting,
> and it was too hard to make that happen.  So now I look with great
> suspicion at anything that complicates our transactional behavior.)
>
> >> I would also like to propose a server option, inherit_deferrable, to
> >> open the remote transactions in deferrable mode if the local
> >> transaction is deferrable.
>
> > The documentation about deferrable is quite confusing. It says "The
> > DEFERRABLE transaction property has no effect unless the transaction
> > is also SERIALIZABLE and READ ONLY." But it doesn't tell what's the
> > effect of deferrable transaction. But probably we don't need a server
> > option here as well.
>
> Yeah, same with this: we should either change it or not.  Multiple
> possible transactional behaviors don't do anyone any favors.

I thought some users might rely on the current behavior that remote
transactions can write even if the local transaction is READ ONLY, so
it would be a good idea to add a server option for backwards
compatibility, but I agree that such an option would cause another,
more difficult problem you mentioned above.

I think the current behavior is not easy to understand, causing
unexpected results in a READ ONLY transaction as shown upthread, so I
would like to instead propose to fix it (for HEAD only as there are no
reports on this issue if I remember correctly).  I think those relying
on the current behavior should just re-declare their transactions as
READ WRITE.  Attached is an updated version of the patch, which fixes
the deferrable case as well.

In the patch I also fixed a bug; I trusted XactReadOnly to see if the
local transaction is READ ONLY, but I noticed that that is not 100%
correct, because a transaction which started as READ WRITE can show as
READ ONLY later within subtransactions, so I modified the patch so
that postgres_fdw opens remote transactions in READ ONLY mode if the
local transaction has been declared READ ONLY at the top level.

Thank you both!

Best regards,
Etsuro Fujita