[PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
29 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Gilles Darold-2

Hi,


As per the following code, t1 is a remote table through postgres_fdw:


test=# BEGIN;
BEGIN
test=# SELECT * FROM t1;
...

test=# PREPARE TRANSACTION 'gxid1';
ERROR:  cannot prepare a transaction that modified remote tables


I have attached a patch to the documentation that adds remote tables to the list of objects where any operation prevent using a prepared transaction, currently it is just notified "operations involving temporary tables or the session's temporary namespace".


The second patch modify the message returned by postgres_fdw as per the SELECT statement above the message should be more comprehensible with:

    ERROR:  cannot PREPARE a transaction that has operated on remote tables

like for temporary objects:

    ERROR:  cannot PREPARE a transaction that has operated on temporary objects


Best regards,

--

Gilles

-- 
Gilles Darold
http://www.darold.net/

fix_prepare_transaction_doc-v1.diff (998 bytes) Download Attachment
fix_message_pg_fdw_v1.diff (614 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Michael Paquier-2
On Fri, Nov 01, 2019 at 05:29:23PM +0100, Gilles Darold wrote:
> I have attached a patch to the documentation that adds remote tables to
> the list of objects where any operation prevent using a prepared
> transaction, currently it is just notified "operations involving
> temporary tables or the session's temporary namespace".

Perhaps we had better use foreign tables for the error message and the
docs?
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Gilles Darold-3
Le 02/11/2019 à 08:31, Michael Paquier a écrit :
> On Fri, Nov 01, 2019 at 05:29:23PM +0100, Gilles Darold wrote:
>> I have attached a patch to the documentation that adds remote tables to
>> the list of objects where any operation prevent using a prepared
>> transaction, currently it is just notified "operations involving
>> temporary tables or the session's temporary namespace".
> Perhaps we had better use foreign tables for the error message and the
> docs?
> --
> Michael


Agree, attached is a new version of the patches that replace word remote
by foreign.

--

Gilles


fix_message_pg_fdw_v2.diff (606 bytes) Download Attachment
fix_prepare_transaction_doc-v2.diff (973 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
In reply to this post by Gilles Darold-2
Hi Gilles,

On Sat, Nov 2, 2019 at 1:29 AM Gilles Darold <[hidden email]> wrote:
> As per the following code, t1 is a remote table through postgres_fdw:

> test=# BEGIN;
> BEGIN
> test=# SELECT * FROM t1;
> ...
>
> test=# PREPARE TRANSACTION 'gxid1';
> ERROR:  cannot prepare a transaction that modified remote tables

> I have attached a patch to the documentation that adds remote tables to the list of objects where any operation prevent using a prepared transaction, currently it is just notified "operations involving temporary tables or the session's temporary namespace".

I'm not sure that's a good idea because file_fdw works well for
PREPARE TRANSACTION!  How about adding a note about  that to the
section of Transaction Management in the postgres_fdw documentation
like the attached?

> The second patch modify the message returned by postgres_fdw as per the SELECT statement above the message should be more comprehensible with:
>
>     ERROR:  cannot PREPARE a transaction that has operated on remote tables
>
> like for temporary objects:
>
>     ERROR:  cannot PREPARE a transaction that has operated on temporary objects

+1  (I too think it would be better to use "foreign tables" rather
than "remote tables" as pointed by Michael-san, but I think it might
be much better to use "postgres_fdw foreign tables", not just "foreign
tables".)

Best regards,
Etsuro Fujita

improve-postgres-fdw-doc.patch (738 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Gilles Darold-2
Hi Esturo,

Le 05/11/2019 à 10:35, Etsuro Fujita a écrit :

> Hi Gilles,
>
> On Sat, Nov 2, 2019 at 1:29 AM Gilles Darold <[hidden email]> wrote:
>> As per the following code, t1 is a remote table through postgres_fdw:
>> test=# BEGIN;
>> BEGIN
>> test=# SELECT * FROM t1;
>> ...
>>
>> test=# PREPARE TRANSACTION 'gxid1';
>> ERROR:  cannot prepare a transaction that modified remote tables
>> I have attached a patch to the documentation that adds remote tables to the list of objects where any operation prevent using a prepared transaction, currently it is just notified "operations involving temporary tables or the session's temporary namespace".
> I'm not sure that's a good idea because file_fdw works well for
> PREPARE TRANSACTION!  How about adding a note about  that to the
> section of Transaction Management in the postgres_fdw documentation
> like the attached?

You are right, read only FDW can be used. A second point in favor of
your remark is that this is the responsibility of the FDW implementation
to throw an error when used with a prepared transaction and I see that
this is not the case for all FDW.


I have attached a single patch that include Etsuro Fujita's patch on
postgres_fdw documentation and mine on postgres_fdw error message with
the precision that it comes from postgres_fdw. The modification about
prepared transaction in PostgreSQL documentation has been removed.


--
Gilles Darold


fix_postgres_fdw_prepared_transaction-v3.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
Hi Gilles,

On Tue, Nov 5, 2019 at 8:41 PM Gilles Darold <[hidden email]> wrote:
> I have attached a single patch that include Etsuro Fujita's patch on
> postgres_fdw documentation and mine on postgres_fdw error message with
> the precision that it comes from postgres_fdw. The modification about
> prepared transaction in PostgreSQL documentation has been removed.

Thanks for the patch!  I added the commit message.  Does that make
sense?  If there are no objections, I'll apply the patch to all
supported branches.

Best regards,
Etsuro Fujita

fix_postgres_fdw_prepared_transaction-v3-efujita.diff (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Michael Paquier-2
On Wed, Nov 06, 2019 at 12:57:10PM +0900, Etsuro Fujita wrote:
> Thanks for the patch!  I added the commit message.  Does that make
> sense?  If there are no objections, I'll apply the patch to all
> supported branches.

"postgres_fdw foreign tables" sounds weird to me.  Could "foreign
tables using postgres_fdw" be a better wording?  I am wondering as
well if we should not split this information into two parts: one for
the actual error message which only mentions foreign tables, and a
second one with a hint to mention that postgres_fdw has been used.

We could have more test cases with 2PC in this module, not necessarily
the responsibility of this patch, but while we're on it..
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
Hi Michael-san,

On Wed, Nov 6, 2019 at 1:13 PM Michael Paquier <[hidden email]> wrote:
> "postgres_fdw foreign tables" sounds weird to me.  Could "foreign
> tables using postgres_fdw" be a better wording?  I am wondering as
> well if we should not split this information into two parts: one for
> the actual error message which only mentions foreign tables, and a
> second one with a hint to mention that postgres_fdw has been used.

We use "postgres_fdw foreign tables" or "postgres_fdw tables" in
release notes, so I thought it was OK to use that in error messages as
well.  But actually, these wordings are not suitable for error
messages?

> We could have more test cases with 2PC in this module, not necessarily
> the responsibility of this patch, but while we're on it..

Agreed.  Will do.

Best regards,
Etsuro Fujita


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Michael Paquier-2
On Wed, Nov 06, 2019 at 03:12:04PM +0900, Etsuro Fujita wrote:

> On Wed, Nov 6, 2019 at 1:13 PM Michael Paquier <[hidden email]> wrote:
>> "postgres_fdw foreign tables" sounds weird to me.  Could "foreign
>> tables using postgres_fdw" be a better wording?  I am wondering as
>> well if we should not split this information into two parts: one for
>> the actual error message which only mentions foreign tables, and a
>> second one with a hint to mention that postgres_fdw has been used.
>
> We use "postgres_fdw foreign tables" or "postgres_fdw tables" in
> release notes, so I thought it was OK to use that in error messages as
> well.  But actually, these wordings are not suitable for error
> messages?
It is true that the docs of postgres_fdw use that and that it is used
in some comments.  Still, I found this wording a bit weird..  If you
think that what you have is better, I am also fine to let you have the
final word, so please feel to ignore me :)
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
Hi Michael-san,

On Wed, Nov 6, 2019 at 4:35 PM Michael Paquier <[hidden email]> wrote:

> On Wed, Nov 06, 2019 at 03:12:04PM +0900, Etsuro Fujita wrote:
> > On Wed, Nov 6, 2019 at 1:13 PM Michael Paquier <[hidden email]> wrote:
> >> "postgres_fdw foreign tables" sounds weird to me.  Could "foreign
> >> tables using postgres_fdw" be a better wording?  I am wondering as
> >> well if we should not split this information into two parts: one for
> >> the actual error message which only mentions foreign tables, and a
> >> second one with a hint to mention that postgres_fdw has been used.
> >
> > We use "postgres_fdw foreign tables" or "postgres_fdw tables" in
> > release notes, so I thought it was OK to use that in error messages as
> > well.  But actually, these wordings are not suitable for error
> > messages?
>
> It is true that the docs of postgres_fdw use that and that it is used
> in some comments.  Still, I found this wording a bit weird..  If you
> think that what you have is better, I am also fine to let you have the
> final word, so please feel to ignore me :)

I'd like to hear the opinions of others.

Best regards,
Etsuro Fujita


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Kyotaro Horiguchi-4
Hello.

At Wed, 6 Nov 2019 20:13:10 +0900, Etsuro Fujita <[hidden email]> wrote in

> Hi Michael-san,
>
> On Wed, Nov 6, 2019 at 4:35 PM Michael Paquier <[hidden email]> wrote:
> > On Wed, Nov 06, 2019 at 03:12:04PM +0900, Etsuro Fujita wrote:
> > > On Wed, Nov 6, 2019 at 1:13 PM Michael Paquier <[hidden email]> wrote:
> > >> "postgres_fdw foreign tables" sounds weird to me.  Could "foreign
> > >> tables using postgres_fdw" be a better wording?  I am wondering as
> > >> well if we should not split this information into two parts: one for
> > >> the actual error message which only mentions foreign tables, and a
> > >> second one with a hint to mention that postgres_fdw has been used.
> > >
> > > We use "postgres_fdw foreign tables" or "postgres_fdw tables" in
> > > release notes, so I thought it was OK to use that in error messages as
> > > well.  But actually, these wordings are not suitable for error
> > > messages?
> >
> > It is true that the docs of postgres_fdw use that and that it is used
> > in some comments.  Still, I found this wording a bit weird..  If you
> > think that what you have is better, I am also fine to let you have the
> > final word, so please feel to ignore me :)
>
> I'd like to hear the opinions of others.

FWIW, I see it a bit weird, too. And perhaps "prepare" should be in
upper case letters. Plus, any operation including a SELECT on a
temporary table inhibits PREAPRE TRANSACTION, but the same on a
postgres_fdw foreign tables is not. So the error message is rather
wrong.

A verbose alternative can be:

"cannot PREPARE a transaction that has modified data on foreign tables using postgres_fdw"

Or I think different style is OK here since the message is not of core
but of an extension.

"postgres_fdw doesn't support PREPARE of a transaction that has modified data on foreign tables"

That could be shorter or simpler, of course.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Gilles Darold-2
Hi Kyotaro,

Le 07/11/2019 à 08:10, Kyotaro Horiguchi a écrit :

> Hello.
>
> At Wed, 6 Nov 2019 20:13:10 +0900, Etsuro Fujita <[hidden email]> wrote in
>> Hi Michael-san,
>>
>> On Wed, Nov 6, 2019 at 4:35 PM Michael Paquier <[hidden email]> wrote:
>>> On Wed, Nov 06, 2019 at 03:12:04PM +0900, Etsuro Fujita wrote:
>>>> On Wed, Nov 6, 2019 at 1:13 PM Michael Paquier <[hidden email]> wrote:
>>>>> "postgres_fdw foreign tables" sounds weird to me.  Could "foreign
>>>>> tables using postgres_fdw" be a better wording?  I am wondering as
>>>>> well if we should not split this information into two parts: one for
>>>>> the actual error message which only mentions foreign tables, and a
>>>>> second one with a hint to mention that postgres_fdw has been used.
>>>> We use "postgres_fdw foreign tables" or "postgres_fdw tables" in
>>>> release notes, so I thought it was OK to use that in error messages as
>>>> well.  But actually, these wordings are not suitable for error
>>>> messages?
>>> It is true that the docs of postgres_fdw use that and that it is used
>>> in some comments.  Still, I found this wording a bit weird..  If you
>>> think that what you have is better, I am also fine to let you have the
>>> final word, so please feel to ignore me :)
>> I'd like to hear the opinions of others.
> FWIW, I see it a bit weird, too. And perhaps "prepare" should be in
> upper case letters. Plus, any operation including a SELECT on a
> temporary table inhibits PREAPRE TRANSACTION, but the same on a
> postgres_fdw foreign tables is not. So the error message is rather
> wrong.


This is not what I've experienced, see the first message of the thread.
A SELECT on foreign table prevent to use PREPARE TRANSACTION like with
temporary table. Perhaps postgres_fdw should not throw an error with
readonly queries on foreign tables but I guess that it is pretty hard to
know especially on a later PREPARE event. But maybe I'm wrong, it is not
easy every day :-) Can you share the SQL code you have executed to allow
PREPARE transaction after a SELECT on a postgres_fdw foreign table?


--
Gilles Darold




Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
In reply to this post by Kyotaro Horiguchi-4
Horiguchi-san,

On Thu, Nov 7, 2019 at 4:11 PM Kyotaro Horiguchi
<[hidden email]> wrote:

> At Wed, 6 Nov 2019 20:13:10 +0900, Etsuro Fujita <[hidden email]> wrote in
> > On Wed, Nov 6, 2019 at 4:35 PM Michael Paquier <[hidden email]> wrote:
> > > On Wed, Nov 06, 2019 at 03:12:04PM +0900, Etsuro Fujita wrote:
> > > > On Wed, Nov 6, 2019 at 1:13 PM Michael Paquier <[hidden email]> wrote:
> > > >> "postgres_fdw foreign tables" sounds weird to me.  Could "foreign
> > > >> tables using postgres_fdw" be a better wording?  I am wondering as
> > > >> well if we should not split this information into two parts: one for
> > > >> the actual error message which only mentions foreign tables, and a
> > > >> second one with a hint to mention that postgres_fdw has been used.
> > > >
> > > > We use "postgres_fdw foreign tables" or "postgres_fdw tables" in
> > > > release notes, so I thought it was OK to use that in error messages as
> > > > well.  But actually, these wordings are not suitable for error
> > > > messages?
> > >
> > > It is true that the docs of postgres_fdw use that and that it is used
> > > in some comments.  Still, I found this wording a bit weird..  If you
> > > think that what you have is better, I am also fine to let you have the
> > > final word, so please feel to ignore me :)
> >
> > I'd like to hear the opinions of others.
>
> FWIW, I see it a bit weird, too.

Only two people complaining about the wording?  Considering as well
that we use that wording in the docs and there were no complains about
that IIRC, I don't feel a need to change that, TBH.

> And perhaps "prepare" should be in
> upper case letters.

Seems like a good idea.

> Plus, any operation including a SELECT on a
> temporary table inhibits PREAPRE TRANSACTION, but the same on a
> postgres_fdw foreign tables is not. So the error message is rather
> wrong.
>
> A verbose alternative can be:
>
> "cannot PREPARE a transaction that has modified data on foreign tables using postgres_fdw"

I don't think that's better, because that doesn't address the original
issue reported in this thread, as Gilles pointed out just before in
his email.  See the commit message in the patch I posted.

Thanks for the comments!

Best regards,
Etsuro Fujita


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Kyotaro Horiguchi-4
In reply to this post by Gilles Darold-2
Hello Gilles. I made a silly mistake.

At Thu, 7 Nov 2019 09:05:55 +0100, Gilles Darold <[hidden email]> wrote in

> > FWIW, I see it a bit weird, too. And perhaps "prepare" should be in
> > upper case letters. Plus, any operation including a SELECT on a
> > temporary table inhibits PREAPRE TRANSACTION, but the same on a
> > postgres_fdw foreign tables is not. So the error message is rather
> > wrong.
>
>
> This is not what I've experienced, see the first message of the thread.
> A SELECT on foreign table prevent to use PREPARE TRANSACTION like with
> temporary table. Perhaps postgres_fdw should not throw an error with
> readonly queries on foreign tables but I guess that it is pretty hard to
> know especially on a later PREPARE event. But maybe I'm wrong, it is not
> easy every day :-) Can you share the SQL code you have executed to allow
> PREPARE transaction after a SELECT on a postgres_fdw foreign table?

Oooops!

After reading this, I came to be afraid that I did something wrong,
then I rechecked actual behavior. Finally I found that SELECT * FROM
foregn_tbl prohibits PREPARE TRANSACTION. I might have used a local
table instead of foreign tabel at the previous trial.

Sorry for the mistake and thank you for pointing it.

So my fixed proposals are:

"cannot PREPARE a transaction that has operated on foreign tables using postgres_fdw"

Or

"postgres_fdw doesn't support PREPARE of a transaction that has accessed foreign tables"

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Kyotaro Horiguchi-4
In reply to this post by Etsuro Fujita-2
Hello, Fujita-san.

At Thu, 7 Nov 2019 17:20:07 +0900, Etsuro Fujita <[hidden email]> wrote in

> Only two people complaining about the wording?  Considering as well
> that we use that wording in the docs and there were no complains about
> that IIRC, I don't feel a need to change that, TBH.
>
> > And perhaps "prepare" should be in
> > upper case letters.
>
> Seems like a good idea.
>
> > Plus, any operation including a SELECT on a
> > temporary table inhibits PREAPRE TRANSACTION, but the same on a
> > postgres_fdw foreign tables is not. So the error message is rather
> > wrong.
> >
> > A verbose alternative can be:
> >
> > "cannot PREPARE a transaction that has modified data on foreign tables using postgres_fdw"
>
> I don't think that's better, because that doesn't address the original
> issue reported in this thread, as Gilles pointed out just before in
> his email.  See the commit message in the patch I posted.

"modified" is my mistake as in the just posted mail. But the most
significant point in the previous mail is using "foreign tables using
postgres_fdw" instead of "postgres_fdw foreign tables". And the other
point is using different message from temporary tables.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Kyotaro Horiguchi-4
At Thu, 07 Nov 2019 17:27:47 +0900 (JST), Kyotaro Horiguchi <[hidden email]> wrote in
> "modified" is my mistake as in the just posted mail. But the most
> significant point in the previous mail is using "foreign tables using
> postgres_fdw" instead of "postgres_fdw foreign tables". And the other
> point is using different message from temporary tables.

I forgot to mention that the comment in XACT_EVENT_PRE_PREPARE
contains the same mistake and needs more or less the same fix.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
In reply to this post by Kyotaro Horiguchi-4
Horiguchi-san,

On Thu, Nov 7, 2019 at 5:28 PM Kyotaro Horiguchi
<[hidden email]> wrote:
> At Thu, 7 Nov 2019 17:20:07 +0900, Etsuro Fujita <[hidden email]> wrote in
> > Only two people complaining about the wording?  Considering as well
> > that we use that wording in the docs and there were no complains about
> > that IIRC, I don't feel a need to change that, TBH.

> But the most
> significant point in the previous mail is using "foreign tables using
> postgres_fdw" instead of "postgres_fdw foreign tables".

OK, but as I said above, I don't feel the need to change that.  How
about leaving it for another patch to improve the wording in that
message and/or the documentation if we really need to do it.

> And the other
> point is using different message from temporary tables.

You mean we should do s/prepare/PREPARE/?

Best regards,
Etsuro Fujita


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Etsuro Fujita-2
In reply to this post by Kyotaro Horiguchi-4
Horiguchi-san,

On Thu, Nov 7, 2019 at 5:31 PM Kyotaro Horiguchi
<[hidden email]> wrote:
> I forgot to mention that the comment in XACT_EVENT_PRE_PREPARE
> contains the same mistake and needs more or less the same fix.

Good catch!  How about rewriting "We disallow remote transactions that
modified anything" in the comment simply to "We disallow any remote
transactions" or something like that?  Attached is an updated patch.
In the patch, I did s/prepare/PREPARE/ to the error message as well,
as you proposed.

Thanks again for reviewing!

Best regards,
Etsuro Fujita

fix_postgres_fdw_prepared_transaction-v4.diff (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Michael Paquier-2
In reply to this post by Etsuro Fujita-2
On Thu, Nov 07, 2019 at 06:40:36PM +0900, Etsuro Fujita wrote:
> On Thu, Nov 7, 2019 at 5:28 PM Kyotaro Horiguchi
> <[hidden email]> wrote:
>> At Thu, 7 Nov 2019 17:20:07 +0900, Etsuro Fujita <[hidden email]> wrote in
>>> Only two people complaining about the wording?  Considering as well

That's like..  Half the folks participating to this thread ;)

>>> that we use that wording in the docs and there were no complains about
>>> that IIRC, I don't feel a need to change that, TBH.
>> But the most
>> significant point in the previous mail is using "foreign tables using
>> postgres_fdw" instead of "postgres_fdw foreign tables".
>
> OK, but as I said above, I don't feel the need to change that.  How
> about leaving it for another patch to improve the wording in that
> message and/or the documentation if we really need to do it.

Fine by me.  If I were to put a number on that, I would be around +-0,
so I don't have an actual objection with your point of view either.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][DOC] Fix for PREPARE TRANSACTION doc and postgres_fdw message.

Gilles Darold-2
In reply to this post by Etsuro Fujita-2
Le 07/11/2019 à 11:52, Etsuro Fujita a écrit :

> Horiguchi-san,
>
> On Thu, Nov 7, 2019 at 5:31 PM Kyotaro Horiguchi
> <[hidden email]> wrote:
>> I forgot to mention that the comment in XACT_EVENT_PRE_PREPARE
>> contains the same mistake and needs more or less the same fix.
> Good catch!  How about rewriting "We disallow remote transactions that
> modified anything" in the comment simply to "We disallow any remote
> transactions" or something like that?  Attached is an updated patch.
> In the patch, I did s/prepare/PREPARE/ to the error message as well,
> as you proposed.
>
> Thanks again for reviewing!
>
> Best regards,
> Etsuro Fujita


Looks good for me,

--
Gilles Darold



12