Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n1cSm-0001KJ-Jf for pgsql-hackers@arkaria.postgresql.org; Sun, 26 Dec 2021 22:58:01 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1n1cSj-0005Vv-Og for pgsql-hackers@arkaria.postgresql.org; Sun, 26 Dec 2021 22:57:57 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n1cSh-0005Vl-Kh for pgsql-hackers@lists.postgresql.org; Sun, 26 Dec 2021 22:57:57 +0000 Received: from new1-smtp.messagingengine.com ([66.111.4.221]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n1cSa-0005Pq-20 for pgsql-hackers@lists.postgresql.org; Sun, 26 Dec 2021 22:57:54 +0000 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailnew.nyi.internal (Postfix) with ESMTP id 6A9F558015E; Sun, 26 Dec 2021 17:57:46 -0500 (EST) Received: from imap49 ([10.202.2.99]) by compute5.internal (MEProxy); Sun, 26 Dec 2021 17:57:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eulerto.com; h= mime-version:message-id:in-reply-to:references:date:from:to:cc :subject:content-type; s=fm3; bh=StrfjkuG/IItvP0wMjYZ2kokka23iM/ 7ry/1Tx/mRxQ=; b=ZBU4lAVDlllRhM9q3XYijP5Iy5QDLQ+QfGBmy9UCqsLmHVH haaLqI6vAyy8Su+QRGOUXX/Zm6Gjegp4M0n4VxCV/byZS2CYqiqHfZIbboFFoXtK etBgmzNQklFLTGF9pbQQYMYIL5wMb3KT25iYvaaSbt1kWhXZlu6fhL+fwgfwfgQG he9npvH8Iqyf3B4ZjuxSQ+7OLQfOQH6b6i7eI3LpY+QgweaJar256pwNcZw9WO80 qtbtX+dRacGeEf7c5SNZViv132bCmJO3w/HDNej6HwO3xrP10Hoa5qCFLL7nx9Za vPJGr9TDPNq39lTIkdmNpXI2VD0VCogPrnBz/IQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=Strfjk uG/IItvP0wMjYZ2kokka23iM/7ry/1Tx/mRxQ=; b=Jh4+vL8iYJtHSW/3qiLEkj 6oByo4j5p6tKva5d7RQ3MT5hIWJM17UY7679lunzYIZNVv0L+fJRUePhJsssqm0Y OGrLyvPcgLB20RI0OFfdLSEZEA0DNAYqBUObyr16wKCsa/bps6K1ntD01dE/dAGp oxi169eA2dYf5JNYUlJ+A3YgnLUT5mJakJ3Xho9bBx7dRWg0Aj4zp1/+geKUJ+Tr cIobwwrFt/pGnAdWIrwVecz7n5zwwt0MJF3qyYsbvEmixoN/Rmwxo3cgAd4qwgET EycODwooHL67nWYrNaC2/Cx/nJzoyuedqVMiPa5au4pi2aiHq7Phi61L9XLOQQAw == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddruddugedgjeegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvffutgesrgdtreerreertdenucfhrhhomhepfdfguhhl vghrucfvrghvvghirhgrfdcuoegvuhhlvghrsegvuhhlvghrthhordgtohhmqeenucggtf frrghtthgvrhhnpeejtefhuedvgffftdetvedufedtjeeuffekleevkefhveekgeelfefh jeehtdejgeenucffohhmrghinhepvghnthgvrhhprhhishgvuggsrdgtohhmnecuvehluh hsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepvghulhgvrhesvghu lhgvrhhtohdrtghomh X-ME-Proxy: Received: by mailuser.nyi.internal (Postfix, from userid 501) id A670FF60074; Sun, 26 Dec 2021 17:57:45 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.5.0-alpha0-4525-g8883000b21-fm-20211221.001-g8883000b Mime-Version: 1.0 Message-Id: In-Reply-To: <202112261609.qp3j4dz2ha2b@alvherre.pgsql> References: <202112261609.qp3j4dz2ha2b@alvherre.pgsql> Date: Sun, 26 Dec 2021 19:57:24 -0300 From: "Euler Taveira" To: "Alvaro Herrera" Cc: "Amit Kapila" , "Peter Smith" , "Ajin Cherian" , "houzj.fnst@fujitsu.com" , "Greg Nancarrow" , "vignesh C" , "tanghy.fnst@fujitsu.com" , "Dilip Kumar" , "Rahila Syed" , "Peter Eisentraut" , =?UTF-8?Q?=C3=96nder_Kalac=C4=B1?= , japin , "Michael Paquier" , "David Steele" , "Craig Ringer" , "Amit Langote" , "PostgreSQL Hackers" Subject: Re: row filtering for logical replication Content-Type: multipart/alternative; boundary=33fea36b50834ec6b12d231062a1425c List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --33fea36b50834ec6b12d231062a1425c Content-Type: text/plain On Sun, Dec 26, 2021, at 1:09 PM, Alvaro Herrera wrote: > On 2021-Dec-26, Euler Taveira wrote: > > > On Sat, Dec 25, 2021, at 1:20 AM, Amit Kapila wrote: > > > On Fri, Dec 24, 2021 at 11:04 AM Peter Smith wrote: > > > > > > > > So, IMO the PG docs wording for this part should be relaxed a bit. > > > > > > > > e.g. > > > > BEFORE: > > > > + A nullable column in the WHERE clause could cause the > > > > + expression to evaluate to false; avoid using columns without not-null > > > > + constraints in the WHERE clause. > > > > AFTER: > > > > + A nullable column in the WHERE clause could cause the > > > > + expression to evaluate to false. To avoid unexpected results, any possible > > > > + null values should be accounted for. > > Is this actually correct? I think a null value would cause the > expression to evaluate to null, not false; the issue is that the filter > considers a null value as not matching (right?). Maybe it's better to > spell that out explicitly; both these wordings seem distracting. [Reading it again...] I think it is referring to the pgoutput_row_filter_exec_expr() return. That's not accurate because it is talking about the expression and the expression returns true, false and null. However, the referred function returns only true or false. I agree that we should explictily mention that a null return means the row won't be published. > You have this elsewhere: > > + If the optional WHERE clause is specified, only rows > + that satisfy the expression > + will be published. Note that parentheses are required around the > + expression. It has no effect on TRUNCATE commands. > > Maybe this whole thing is clearer if you just say "If the optional WHERE > clause is specified, rows for which the expression returns false or null > will not be published." With that it should be fairly clear what > happens if you have NULL values in the columns used in the expression, > and you can just delete that phrase you're discussing. Your proposal sounds good to me. -- Euler Taveira EDB https://www.enterprisedb.com/ --33fea36b50834ec6b12d231062a1425c Content-Type: text/html Content-Transfer-Encoding: quoted-printable
On Sun, Dec 26,= 2021, at 1:09 PM, Alvaro Herrera wrote:
On 2021-Dec-26, Euler Taveira wrote:

> On Sat, Dec 25, 2021, at 1:20 AM, Amit Kapi= la wrote:
> > On Fri, Dec 24, 2021 at 11:04 AM Peter= Smith <smithpb2250@gmail.co= m> wrote:
> > >
> > &g= t; So, IMO the PG docs wording for this part should be relaxed a bit.
> > >
> > > e.g.
> > > BEFORE:
> > > +   A n= ullable column in the <literal>WHERE</literal> clause could = cause the
> > > +   expression to evalu= ate to false; avoid using columns without not-null
> &g= t; > +   constraints in the <literal>WHERE</litera= l> clause.
> > > AFTER:
> >= ; > +   A nullable column in the <literal>WHERE</l= iteral> clause could cause the
> > > + &n= bsp; expression to evaluate to false. To avoid unexpected results, any p= ossible
> > > +   null values should be= accounted for.

Is this actually correct?&n= bsp; I think a null value would cause the
expression to ev= aluate to null, not false; the issue is that the filter
co= nsiders a null value as not matching (right?).  Maybe it's better t= o
spell that out explicitly; both these wordings seem dist= racting.
[Reading it again...] I think it is = referring to the
pgoutput_row_filter_exec_expr() return. T= hat's not accurate because it is
talking about the express= ion and the expression returns true, false and null.
Howev= er, the referred function returns only true or false. I agree that we
should explictily mention that a null return means the row w= on't be published.

You have this elsewhere:

+      If the optional <literal>WHERE&= lt;/literal> clause is specified, only rows
+ &nbs= p;    that satisfy the <replaceable class=3D"parameter= ">expression</replaceable> 
+  &nb= sp;   will be published. Note that parentheses are required ar= ound the 
+      expression.= It has no effect on <literal>TRUNCATE</literal> commands.

Maybe this whole thing is clearer if you jus= t say "If the optional WHERE
clause is specified, rows for= which the expression returns false or null
will not be pu= blished."  With that it should be fairly clear what
h= appens if you have NULL values in the columns used in the expression,
and you can just delete that phrase you're discussing.
Your proposal sounds good to me.

= --
Euler Taveira
EDB   ht= tps://www.enterprisedb.com/

--33fea36b50834ec6b12d231062a1425c--