Hello,

On 2019-Aug-06, Amit Langote wrote:

> On Mon, Aug 5, 2019 at 11:39 PM Alvaro Herrera <alvhe...@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
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;
 
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        

Reply via email to