pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: adding partitioned tables to publications
Date: Thu, 19 Mar 2020 15:17:57 +0100
Message-ID: <b8f257af-3fcd-65b9-088c-0248d82e4e99@2ndquadrant.com> (raw)
In-Reply-To: <CA+HiwqGVbp0yh-7-FscZiYu1B_CJjaDZRTYkPTjnjpUgSzE6RA@mail.gmail.com>
References: <CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com>
	<b6d8519b-353f-1949-79b5-460a6712e20f@2ndquadrant.com>
	<CA+HiwqHScTtXD97yC_ZRLk7TXf+0HYH+UeDvACf2Q-LBH-2ctw@mail.gmail.com>
	<CA+FpmFfKmsNww_C5hWU-=wg9h3cyV7vZcCUKWrrhtLkteUtY4w@mail.gmail.com>
	<CA+HiwqE2ie6_T01fMzsf7c5=hCGoVdj2yTStCJ78zOmayMVWbw@mail.gmail.com>
	<825b31ea-ad9f-21d1-29b6-2a494513458d@2ndquadrant.com>
	<CA+HiwqEqm3xvnMHjstkrECrpVv9R7YgYYpRHHVrffSvA9Zrf9g@mail.gmail.com>
	<CA+HiwqGiFpkHxx0mc2zpdJLGzfqEaqsdqUZurNto9kbkwzcQ1w@mail.gmail.com>
	<CA+FpmFfve1TtLvj5CwpgEZrjNiFG-EWh4ywpbtrB7-UTkZo2gg@mail.gmail.com>
	<CA+HiwqHfV73E4aJxX6d79xSEZjp+stFKnD0+7OktPw2_tD=pYg@mail.gmail.com>
	<73a1877a-6f1d-9f2e-da51-bc2eb57e07d5@2ndquadrant.com>
	<CA+HiwqFjo=vCXDdkwprf6ipaVeegCpW79n-Ds8BK5cNQsSey2A@mail.gmail.com>
	<d760bb60-563c-dd01-9903-f9337d50e8a6@2ndquadrant.com>
	<CA+HiwqFBni-cXy5NBN0PdwxidXgcCEio0N_AOnOM0pR2dMWUyA@mail.gmail.com>
	<CA+HiwqGVbp0yh-7-FscZiYu1B_CJjaDZRTYkPTjnjpUgSzE6RA@mail.gmail.com>

On 2020-03-18 08:33, Amit Langote wrote:
> By the way, I have rebased the patches, although maybe you've got your
> own copies; attached.

Looking through 0002 and 0003 now.

The structure looks generally good.

In 0002, the naming of apply_handle_insert() vs. 
apply_handle_do_insert() etc. seems a bit prone to confusion.  How about 
something like apply_handle_insert_internal()?  Also, should we put each 
of those internal functions next to their internal function instead of 
in a separate group like you have it?

In apply_handle_do_insert(), the argument localslot should probably be 
remoteslot.

In apply_handle_do_delete(), the ExecOpenIndices() call was moved to a 
different location relative to the rest of the code.  That was probably 
not intended.

In 0003, you have /* TODO, use inverse lookup hashtable? */.  Is this 
something you plan to address in this cycle, or is that more for future 
generations?

0003 could use some more tests.  The one test that you adjusted just 
ensures the data goes somewhere instead of being rejected, but there are 
no tests that check whether it ends up in the right partition, whether 
cross-partition updates work etc.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





view thread (86+ messages)  latest in thread

Message-ID: <b8f257af-3fcd-65b9-088c-0248d82e4e99@2ndquadrant.com>
Permalink:  ../b8f257af-3fcd-65b9-088c-0248d82e4e99@2ndquadrant.com/
Also on:    postgresql.org/message-id/b8f257af-3fcd-65b9-088c-0248d82e4e99@2ndquadrant.com

 · 

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: peter.eisentraut@2ndquadrant.com, amitlangote09@gmail.com, rafia.pghackers@gmail.com
  Subject: Re: adding partitioned tables to publications
  In-Reply-To: <b8f257af-3fcd-65b9-088c-0248d82e4e99@2ndquadrant.com>

* 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