On Fri, Jan 15, 2021 at 4:03 AM Zhihong Yu <z...@yugabyte.com> wrote:
>
> Hi,
> For v32-0008-Prepare-foreign-transactions-at-commit-time.patch :
Thank you for reviewing the patch!
>
> + bool have_notwophase = false;
>
> Maybe name the variable have_no_twophase so that it is easier to read.
Fixed.
>
> + * Two-phase commit is not required if the number of servers performed
>
> performed -> performing
Fixed.
>
> + errmsg("cannot process a distributed transaction that has
> operated on a foreign server that does not support two-phase commit
> protocol"),
> + errdetail("foreign_twophase_commit is \'required\' but the
> transaction has some foreign servers which are not capable of two-phase
> commit")));
>
> The lines are really long. Please wrap into more lines.
Hmm, we can do that but if we do that, it makes grepping by the error
message hard. Please refer to the documentation about the formatting
guideline[1]:
Limit line lengths so that the code is readable in an 80-column
window. (This doesn't mean that you must never go past 80 columns. For
instance, breaking a long error message string in arbitrary places
just to keep the code within 80 columns is probably not a net gain in
readability.)
These changes have been made in the local branch. I'll post the
updated patch set after incorporating all the comments.
Regards,
[1] https://www.postgresql.org/docs/devel/source-format.html
--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/
