pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: movead.li@highgo.ca <movead.li@highgo.ca>
To: michael <michael@paquier.xyz>
Cc: petr <petr@2ndquadrant.com>
Cc: simon <simon@2ndquadrant.com>
Cc: Madan Kumar <madankumar1993@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Cc: craig <craig@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: andres <andres@anarazel.de>
Cc: soumyadeep2007 <soumyadeep2007@gmail.com>
Subject: Re: A patch for get origin from commit_ts.
Date: Wed, 8 Jul 2020 09:31:24 +0800
Message-ID: <2020070809312199165531@highgo.ca> (raw)
References: <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>
	<20200706003635.GC2143@paquier.xyz>
	<202007061112287118394@highgo.ca>
	<20200706080136.GH2143@paquier.xyz>
	<2020070710022545488010@highgo.ca>
	<20200707053548.GA3605@paquier.xyz>


>Cool, thanks.  I have gone through your patch in details, and updated
>it as the attached.  Here are some comments.
 
>'8000' as OID for the new function was not really random, so to be
>fair with the other patches, I picked up the first random value
>unused_oids has given me instead.
> 
>There were some indentation issues, and pgindent got that fixed.
 
>I think that it is better to use "replication origin" in the docs
>instead of just origin.  I have kept "origin" in the functions for
>now as that sounded cleaner to me, but we may consider using something
>like "reporigin" as well as attribute name.
> 
>The tests could just use tstzrange() to make sure that the timestamps
>have valid values, so I have switched to that, and did not resist to
>do the same in the existing tests.
> 
>+-- Test when it can not find the transaction
>+SELECT * FROM pg_xact_commit_timestamp_origin((:'txid_set_origin'::text::int +
>10)::text::xid) x;
>This test could become unstable, particularly if it gets used in a
>parallel environment, so I have removed it.  Perhaps I am just
>over-pessimistic here though..
> 
>As a side note, I think that we could just remove the alternate output
>of commit_ts/, as it does not really get used because of the
>NO_INSTALLCHECK present in the module's Makefile.  That would be the
>job of a different patch, so I have updated it accordingly.  Glad to
>see that you did not forget to adapt it in your own patch.
> 
>(The change in catversion.h is a self-reminder...)
Thanks for all of that, so many details I still need to pay attention when 
submit a patch.



Regards,
Highgo Software (Canada/China/Pakistan) 
URL : www.highgo.ca 
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca


view thread (31+ messages)  latest in thread

Message-ID: <2020070809312199165531@highgo.ca>
Permalink:  ../2020070809312199165531@highgo.ca/
Also on:    postgresql.org/message-id/2020070809312199165531@highgo.ca

 · 

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: movead.li@highgo.ca, michael@paquier.xyz, 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: <2020070809312199165531@highgo.ca>

* 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