pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: movead.li@highgo.ca <movead.li@highgo.ca>
Cc: petr <petr@2ndquadrant.com>
Cc: simon <simon@2ndquadrant.com>
Cc: madankumar1993 <madankumar1993@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Cc: craig <craig@2ndquadrant.com>
Cc: alvherre <alvherre@2ndquadrant.com>
Cc: andres <andres@anarazel.de>
Cc: soumyadeep2007 <soumyadeep2007@gmail.com>
Subject: Re: A patch for get origin from commit_ts.
Date: Mon, 6 Jul 2020 09:36:35 +0900
Message-ID: <20200706003635.GC2143@paquier.xyz> (raw)
In-Reply-To: <202007041801258116499@highgo.ca>
References: <2020051116430836450630@highgo.ca>
	<20200512014537.GF88791@paquier.xyz>
	<2020051316291300911447@highgo.ca>
	<CAMtsaPEm47-mD5br8O8fVsZ0PnowtNij4O3UNsuG6Zb7dSCAaQ@mail.gmail.com>
	<CANP8+jJu-bta+4KBRDf2XKsHybDBY1Ew_zvcuoTdhES+ZWLGcQ@mail.gmail.com>
	<20200702015852.GE10408@paquier.xyz>
	<4ee522fa-b549-4a8d-9d9e-cb27be774627@2ndquadrant.com>
	<20200703051059.GB1729@paquier.xyz>
	<202007041801258116499@highgo.ca>

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

Attachments:

  signature.asc (application/pgp-signature, 832B) download

view thread (31+ messages)  latest in thread

Message-ID: <20200706003635.GC2143@paquier.xyz>
Permalink:  ../20200706003635.GC2143@paquier.xyz/
Also on:    postgresql.org/message-id/20200706003635.GC2143@paquier.xyz

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: michael@paquier.xyz, movead.li@highgo.ca, petr@2ndquadrant.com, simon@2ndquadrant.com, madankumar1993@gmail.com, pgsql-hackers@lists.postgresql.org, craig@2ndquadrant.com, alvherre@2ndquadrant.com, andres@anarazel.de, soumyadeep2007@gmail.com
  Subject: Re: A patch for get origin from commit_ts.
  In-Reply-To: <20200706003635.GC2143@paquier.xyz>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox