Re: A patch for get origin from commit_ts.

On Sat, Jul 04, 2020 at 06:01:28PM +0800, movead.li@highgo.ca wrote:
> I make a patch as Michael Paquier described that use a new function to
> return transactionid and origin, and I add a origin version to
> pg_last_committed_xact() too,  now it looks like below:

+SELECT pg_replication_origin_create('test_commit_ts: get_origin_1');
+SELECT pg_replication_origin_create('test_commit_ts: get_origin_2');
+SELECT pg_replication_origin_create('test_commit_ts: get_origin_3');

Why do you need three replication origins to test three times the same
pattern?  Wouldn't one be enough and why don't you check after the
timestamp?  I would also two extra tests: one with a NULL input and an
extra one where the data could not be found.

+   found = TransactionIdGetCommitTsData(xid, &ts, &nodeid);
+
+   if (!found)
+       PG_RETURN_NULL();

This part also looks incorrect to me, I think that you should still
return two tuples, both marked as NULL.  You can do that just by
switching the nulls flags to true for the two values if nothing can be
found.
--
Michael