Re: Transactions involving multiple postgres foreign servers, take 2 - Mailing list pgsql-hackers

Hi Sawada san,

I just had a quick look to 0001 and 0002 patch here is the few suggestions.

patch: v27-0001:

Typo: s/non-temprary/non-temporary
----

patch: v27-0002: (Note:The left-hand number is the line number in the v27-0002 patch):

 138 +PostgreSQL's the global transaction manager (GTM), as a distributed transaction
 139 +participant The registered foreign transactions are tracked until the end of

Full stop "." is missing after "participant"


174 +API Contract With Transaction Management Callback Functions

Can we just say "Transaction Management Callback Functions"; 

TOBH, I am not sure that I understand this title.

 203 +processing foreign transaction (i.g. preparing, committing or aborting) the

Do you mean "i.e" instead of i.g. ?


269 + * RollbackForeignTransactionAPI. Registered participant servers are identified

Add space before between RollbackForeignTransaction and API.


 292 + * automatically so must be processed manually using by pg_resovle_fdwxact()

Do you mean pg_resolve_foreign_xact() here?


 320 + *   the foreign transaction is authorized to update the fields from its own
 321 + *   one.
 322 +
 323 + * Therefore, before doing PREPARE, COMMIT PREPARED or ROLLBACK PREPARED a

Please add asterisk '*' on line#322.


 816 +static void
 817 +FdwXactPrepareForeignTransactions(void)
 818 +{
 819 +   ListCell        *lcell;

Let's have this variable name as "lc" like elsewhere.


1036 +           ereport(ERROR, (errmsg("could not insert a foreign transaction entry"),
1037 +                           errdetail("duplicate entry with transaction id %u, serverid %u, userid %u",
1038 +                                  xid, serverid, userid)));
1039 +   }

Incorrect formatting.

 
1166 +/*
1167 + * Return true and set FdwXactAtomicCommitReady to true if the current transaction

Do you mean ForeignTwophaseCommitIsRequired instead of FdwXactAtomicCommitReady?


3529 +
3530 +/*
3531 + * FdwXactLauncherRegister
3532 + *     Register a background worker running the foreign transaction
3533 + *      launcher.
3534 + */

This prolog style is not consistent with the other function in the file.


And here are the few typos:

s/conssitent/consistent
s/consisnts/consist
s/Foriegn/Foreign
s/tranascation/transaction
s/itselft/itself
s/rolbacked/rollbacked
s/trasaction/transaction
s/transactio/transaction
s/automically/automatically
s/CommitForeignTransaciton/CommitForeignTransaction
s/Similary/Similarly
s/FDWACT_/FDWXACT_
s/dink/disk
s/requried/required
s/trasactions/transactions
s/prepread/prepared
s/preapred/prepared
s/beging/being
s/gxact/xact
s/in-dbout/in-doubt
s/respecitively/respectively
s/transction/transaction
s/idenetifier/identifier
s/identifer/identifier
s/checkpoint'S/checkpoint's
s/fo/of
s/transcation/transaction
s/trasanction/transaction
s/non-temprary/non-temporary
s/resovler_internal.h/resolver_internal.h

Regards,

Amul