Date: Mon, 3 Jan 2022 08:40:57 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Amit Kapila <amit.kapila16@gmail.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	"houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>,
	Rahila Syed <rahilasyed90@gmail.com>,
	Peter Smith <smithpb2250@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: Column Filtering in Logical Replication
Message-ID: <20220103144056.GB24477@telsasoft.com>
References: <20211231163227.GV24477@telsasoft.com>
 <202201031431.akwhcx6xrusu@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202201031431.akwhcx6xrusu@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220103144056.GB24477%40telsasoft.com>
Precedence: bulk

On Mon, Jan 03, 2022 at 11:31:39AM -0300, Alvaro Herrera wrote:
> > Did the DatumGetBool issue expose a deficiency in testing ?
> > I think the !am_partition path was never being hit.
> 
> Hmm, the TAP test creates a subscription that contains both types of
> tables.  I tried adding an assert for each case, and they were both hit
> on running the test.

Yes, I know both paths are hit now that it uses GetBool.

What I'm wondering is why tests didn't fail when one path wasn't hit - when it
said am_partition=DatumGetChar(); if (!am_partition){}

I suppose it's because the am_partition=true case correctly handles
nonpartitions.

Maybe the !am_partition case should be removed, and add a comment that
pg_partition_tree(pg_partition_root(%u))) also handles non-partitions.
Or maybe that's inefficient...

-- 
Justin