Date: Sun, 4 Aug 2019 18:43:49 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: yuzuko <yuzukohosoya@gmail.com>
Cc: 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: <20190804224349.GA1366@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="RnlQjJ0d97Da+TV1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAKkQ50-7g6EHucX-rh9HpE1yHVPwa-XWhNcEOTHpOWk9RGWi7Q@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--RnlQjJ0d97Da+TV1
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

I propose the comment rewordings as attached.  Mostly, this updates the
comment atop the function to cover the case being modified, and then the
new comment just refers to the new explicitly stated policy, so it
bcomes simpler.

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

--RnlQjJ0d97Da+TV1
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v6_ignore_contradictory_where_clauses_at_partprune_step.patch"

diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 09c4ed83b6..9462264e84 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -849,10 +849,11 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps)
  * the context's steps list.  Each step is assigned a step identifier, unique
  * even across recursive calls.
  *
- * If we find clauses that are mutually contradictory, or a pseudoconstant
- * clause that contains false, we set context->contradictory to true and
- * return NIL (that is, no pruning steps).  Caller should consider all
- * partitions as pruned in that case.
+ * If we find clauses that are mutually contradictory, or contradictory with
+ * the partitioning constraint, or a pseudoconstant clause that contains
+ * false, we set context->contradictory to true and return NIL (that is, no
+ * pruning steps).  Caller should consider all partitions as pruned in that
+ * case.
  */
 static List *
 gen_partprune_steps_internal(GeneratePruningStepsContext *context,
@@ -942,35 +943,15 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
 					}
 					else
 					{
+						PartitionPruneStep *orstep;
+
 						/*
 						 * The arg didn't contain a clause matching this
 						 * partition key.  We cannot prune using such an arg.
 						 * To indicate that to the pruning code, we must
 						 * construct a dummy PartitionPruneStepCombine whose
 						 * source_stepids is set to an empty List.
-						 *
-						 * However, if we can prove using constraint exclusion
-						 * that the clause refutes the table's partition
-						 * constraint (if it's sub-partitioned), we need not
-						 * bother with that.  That is, we effectively ignore
-						 * this OR arm.
 						 */
-						List	   *partconstr = context->rel->partition_qual;
-						PartitionPruneStep *orstep;
-
-						if (partconstr)
-						{
-							partconstr = (List *)
-								expression_planner((Expr *) partconstr);
-							if (context->rel->relid != 1)
-								ChangeVarNodes((Node *) partconstr, 1,
-											   context->rel->relid, 0);
-							if (predicate_refuted_by(partconstr,
-													 list_make1(arg),
-													 false))
-								continue;
-						}
-
 						orstep = gen_prune_step_combine(context, NIL,
 														PARTPRUNE_COMBINE_UNION);
 						arg_stepids = lappend_int(arg_stepids, orstep->step_id);
@@ -1047,6 +1028,28 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context,
 			bool		clause_is_not_null = false;
 			PartClauseInfo *pc = NULL;
 			List	   *clause_steps = NIL;
+			List	   *partconstr = context->rel->partition_qual;
+
+			/*
+			 * 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.
+			 */
+			if (partconstr)
+			{
+				partconstr = (List *)
+					expression_planner((Expr *) partconstr);
+				if (context->rel->relid != 1)
+					ChangeVarNodes((Node *) partconstr, 1,
+								   context->rel->relid, 0);
+				if (predicate_refuted_by(partconstr,
+										 list_make1(clause),
+										 false))
+				{
+					context->contradictory = true;
+					return NIL;
+				}
+			}
 
 			switch (match_clause_to_partition_key(context,
 												  clause, partkey, i,
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 2d3229fd73..6ccc91d390 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -592,6 +592,24 @@ explain (costs off) select * from rlp where a < 1 or (a > 20 and a < 25);
          Filter: ((a < 1) OR ((a > 20) AND (a < 25)))
 (5 rows)
 
+-- where clause contradicts sub-partition's constraint
+explain (costs off) select * from rlp where a = 20 or a = 40;
+               QUERY PLAN               
+----------------------------------------
+ Append
+   ->  Seq Scan on rlp4_1
+         Filter: ((a = 20) OR (a = 40))
+   ->  Seq Scan on rlp5_default
+         Filter: ((a = 20) OR (a = 40))
+(5 rows)
+
+explain (costs off) select * from rlp3 where a = 20;   /* empty */
+        QUERY PLAN        
+--------------------------
+ Result
+   One-Time Filter: false
+(2 rows)
+
 -- redundant clauses are eliminated
 explain (costs off) select * from rlp where a > 1 and a = 10;	/* only default */
             QUERY PLAN            
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index efdedaaeb8..a26c2f0e72 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -85,6 +85,10 @@ explain (costs off) select * from rlp where a = 29;
 explain (costs off) select * from rlp where a >= 29;
 explain (costs off) select * from rlp where a < 1 or (a > 20 and a < 25);
 
+-- where clause contradicts sub-partition's constraint
+explain (costs off) select * from rlp where a = 20 or a = 40;
+explain (costs off) select * from rlp3 where a = 20;   /* empty */
+
 -- redundant clauses are eliminated
 explain (costs off) select * from rlp where a > 1 and a = 10;	/* only default */
 explain (costs off) select * from rlp where a > 1 and a >=15;	/* rlp3 onwards, including default */

--RnlQjJ0d97Da+TV1--