Date: Tue, 6 Aug 2019 13:17:40 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: yuzuko <yuzukohosoya@gmail.com>, shawn wang <shawn.wang.pg@gmail.com>,
	Shawn Wang <shawn.wang@highgo.ca>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Problem with default partition pruning
Message-ID: <20190806171740.GA7610@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="/9DWx/yDrRhgMJTb"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+HiwqGmSnaSdQuTJbOjEjHm4THCuxc9=2EGV4aqW1DHmE3oYQ@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--/9DWx/yDrRhgMJTb
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hello,

On 2019-Aug-06, Amit Langote wrote:

> On Mon, Aug 5, 2019 at 11:39 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

> > I don't think that we care about what happens with constraint_exclusion
> > is on.  That's not the recommended value for that setting anyway.
> 
> One issue I expressed with unconditionally applying constraint
> exclusion in partprune.c the way the third patch proposes to do it is
> that it means we end up performing the same processing twice for a
> given relation in come cases.  Specifically, when constraint_exclusion
> is set to on, relation_excluded_by_constraints() that occurs when
> set_rel_sizes() is applied to that relation would perform the same
> proof.  But maybe we shouldn't worry about the repetition too much,
> because it's not likely to be very problematic in practice,
> considering that setting constraint_exclusion to on is not
> recommended.

Well, if this is really all that duplicative, one thing we could do is
run this check in get_partprune_steps_internal only if
constraint_exclusion is a value other than on; we should achieve the
same effect with no repetition.  Patch for that is attached.  However,
if I run the server with constraint_exclusion=on, the regression test
fail with the attached diff.  I didn't look at what test is failing, but
it seems to me that it's not really duplicative in all cases, only some.
Therefore we can't do it.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--/9DWx/yDrRhgMJTb
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="no-dupe-check.patch"

diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 2ed1e44c18..2f84f30423 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1023,8 +1023,12 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
 		 * If the clause contradicts the partition constraint, mark the clause
 		 * as contradictory and we're done.  This is particularly helpful to
 		 * prune the default partition.
+		 *
+		 * However, we skip this if constraint_exclusion is set to ON, because
+		 * this check will be done by set_rel_size later on anyway.
 		 */
-		if (context->rel->partition_qual)
+		if (context->rel->partition_qual &&
+			constraint_exclusion != CONSTRAINT_EXCLUSION_ON)
 		{
 			List	   *partconstr;
 

--/9DWx/yDrRhgMJTb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="regression.diffs"

diff -U3 /pgsql/source/master/src/test/regress/expected/partition_prune.out /home/alvherre/Code/pgsql/build/master/src/test/regress/results/partition_prune.out
--- /pgsql/source/master/src/test/regress/expected/partition_prune.out	2019-08-06 12:27:10.041137540 -0400
+++ /home/alvherre/Code/pgsql/build/master/src/test/regress/results/partition_prune.out	2019-08-06 13:12:20.367678444 -0400
@@ -529,6 +529,12 @@
          Filter: ((a = 1) OR ((b)::text = 'ab'::text))
    ->  Seq Scan on rlp3abcd
          Filter: ((a = 1) OR ((b)::text = 'ab'::text))
+   ->  Seq Scan on rlp3efgh
+         Filter: ((a = 1) OR ((b)::text = 'ab'::text))
+   ->  Seq Scan on rlp3nullxy
+         Filter: ((a = 1) OR ((b)::text = 'ab'::text))
+   ->  Seq Scan on rlp3_default
+         Filter: ((a = 1) OR ((b)::text = 'ab'::text))
    ->  Seq Scan on rlp4_1
          Filter: ((a = 1) OR ((b)::text = 'ab'::text))
    ->  Seq Scan on rlp4_2
@@ -547,7 +553,7 @@
          Filter: ((a = 1) OR ((b)::text = 'ab'::text))
    ->  Seq Scan on rlp_default_default
          Filter: ((a = 1) OR ((b)::text = 'ab'::text))
-(25 rows)
+(31 rows)
 
 explain (costs off) select * from rlp where a > 20 and a < 27;
                QUERY PLAN                
@@ -599,9 +605,11 @@
  Append
    ->  Seq Scan on rlp4_1
          Filter: ((a = 20) OR (a = 40))
+   ->  Seq Scan on rlp4_default
+         Filter: ((a = 20) OR (a = 40))
    ->  Seq Scan on rlp5_default
          Filter: ((a = 20) OR (a = 40))
-(5 rows)
+(7 rows)
 
 explain (costs off) select * from rlp3 where a = 20;   /* empty */
         QUERY PLAN        

--/9DWx/yDrRhgMJTb--