Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hBBZK-00019h-Te for pgsql-hackers@arkaria.postgresql.org; Tue, 02 Apr 2019 05:02:43 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hBBZJ-0006tz-BU for pgsql-hackers@arkaria.postgresql.org; Tue, 02 Apr 2019 05:02:41 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hBBZI-0006qI-R0 for pgsql-hackers@lists.postgresql.org; Tue, 02 Apr 2019 05:02:41 +0000 Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]) by makus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hBBZE-00088I-NS for pgsql-hackers@lists.postgresql.org; Tue, 02 Apr 2019 05:02:39 +0000 Received: from vc1.ecl.ntt.co.jp (vc1.ecl.ntt.co.jp [129.60.86.153]) by tama50.ecl.ntt.co.jp (8.13.8/8.13.8) with ESMTP id x3252RU4017506; Tue, 2 Apr 2019 14:02:27 +0900 Received: from vc1.ecl.ntt.co.jp (localhost [127.0.0.1]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id 5352BEA7D3C; Tue, 2 Apr 2019 14:02:27 +0900 (JST) Received: from jcms-pop11.ecl.ntt.co.jp (jcms-pop11.ecl.ntt.co.jp [129.60.87.132]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id 48A57EA7C45; Tue, 2 Apr 2019 14:02:27 +0900 (JST) Received: from hosoyahp (unknown [129.60.241.74]) by jcms-pop11.ecl.ntt.co.jp (Postfix) with ESMTPSA id 3FB7470C0ADC; Tue, 2 Apr 2019 14:02:27 +0900 (JST) From: "Yuzuko Hosoya" References: <00e601d4ca86$932b8bc0$b982a340$@lab.ntt.co.jp> <010e01d4ce68$c0427b10$40c77130$@lab.ntt.co.jp> <0F97FA9ABBDBE54F91744A9B37151A5129A78A@g01jpexmbkw24> <001901d4daf5$1ef4f640$5cdee2c0$@lab.ntt.co.jp> <9bed6b79-f264-6976-b880-e2a5d23e9d85@lab.ntt.co.jp> <003501d4de21$8816f540$9844dfc0$@lab.ntt.co.jp> <9bb31dfe-b0d0-53f3-3ea6-e64b811424cf@lab.ntt.co.jp> <005101d4e074$e24d1e40$a6e75ac0$@lab.ntt.co.jp> <001ea23e-638f-9ab2-143d-2cf1105bf8d6@lab.ntt.co.jp> In-Reply-To: <001ea23e-638f-9ab2-143d-2cf1105bf8d6@lab.ntt.co.jp> Subject: RE: Problem with default partition pruning Date: Tue, 2 Apr 2019 14:02:08 +0900 Message-ID: <008f01d4e911$39aa87a0$acff96e0$@lab.ntt.co.jp> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0090_01D4E95C.A99A1EE0" X-Mailer: Microsoft Outlook 15.0 Content-language: ja Thread-index: AQLwSThp7L0tC61luGAd4dJt3HxY3QJvrgn9AgtGZCUBpM85VAEOZ04CAeo+SK4CDbFrsgGXrNc6AV8LXc4CAoWeVAJ/vK7zAyoITl4B/o26T6MzUQpA X-CC-Mail-RelayStamp: 1 To: "'Amit Langote'" , "'Thibaut'" , "'Imai, Yoshikazu'" Cc: "'PostgreSQL Hackers'" X-TM-AS-MML: disable List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multipart message in MIME format. ------=_NextPart_000_0090_01D4E95C.A99A1EE0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Hi, > Maybe we should have two patches as we seem to be improving two things: > > 1. Patch to fix problems with default partition pruning originally reported by Hosoya-san > > 2. Patch to determine if a given clause contradicts a sub-partitioned table's partition constraint, > fixing problems unearthed by Thibaut's tests I attached the latest patches according to Amit comment. v3_default_partition_pruning.patch fixes default partition pruning problems and ignore_contradictory_where_clauses_at_partprune_step.patch fixes sub-partition problems Thibaut tested. Best regards, Yuzuko Hosoya ------=_NextPart_000_0090_01D4E95C.A99A1EE0 Content-Type: application/octet-stream; name="ignore_contradictory_where_clauses_at_partprune_step.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="ignore_contradictory_where_clauses_at_partprune_step.patch" diff --git a/src/backend/partitioning/partprune.c = b/src/backend/partitioning/partprune.c=0A= index 5cd740a..c45cd48 100644=0A= --- a/src/backend/partitioning/partprune.c=0A= +++ b/src/backend/partitioning/partprune.c=0A= @@ -850,32 +850,13 @@ = gen_partprune_steps_internal(GeneratePruningStepsContext *context,=0A= * an arg. To indicate that to the pruning code, we=0A= * must construct a dummy PartitionPruneStepCombine=0A= * whose source_stepids is set to an empty List.=0A= - * However, if we can prove using constraint exclusion=0A= - * that the clause refutes the table's partition=0A= - * constraint (if it's sub-partitioned), we need not=0A= - * bother with that. That is, we effectively ignore=0A= - * this OR arm.=0A= */=0A= - List *partconstr =3D rel->partition_qual;=0A= PartitionPruneStep *orstep;=0A= =0A= /* Just ignore this argument. */=0A= if (arg_contradictory)=0A= continue;=0A= =0A= - if (partconstr)=0A= - {=0A= - partconstr =3D (List *)=0A= - expression_planner((Expr *) partconstr);=0A= - if (rel->relid !=3D 1)=0A= - ChangeVarNodes((Node *) partconstr, 1,=0A= - rel->relid, 0);=0A= - if (predicate_refuted_by(partconstr,=0A= - list_make1(arg),=0A= - false))=0A= - continue;=0A= - }=0A= -=0A= orstep =3D gen_prune_step_combine(context, NIL,=0A= PARTPRUNE_COMBINE_UNION);=0A= arg_stepids =3D lappend_int(arg_stepids, orstep->step_id);=0A= @@ -951,6 +932,7 @@ = gen_partprune_steps_internal(GeneratePruningStepsContext *context,=0A= bool clause_is_not_null =3D false;=0A= PartClauseInfo *pc =3D NULL;=0A= List *clause_steps =3D NIL;=0A= + List *partconstr =3D rel->partition_qual;=0A= =0A= switch (match_clause_to_partition_key(rel, context,=0A= clause, partkey, i,=0A= @@ -961,6 +943,29 @@ = gen_partprune_steps_internal(GeneratePruningStepsContext *context,=0A= Assert(pc !=3D NULL);=0A= =0A= /*=0A= + * If this clause can be proved false for this partition,=0A= + * given its partition constraint, we can ignore it, =0A= + * that is not try to pass it to the pruning code. =0A= + * We should do that especially to avoid pruning code =0A= + * wrongly failing to prune the default partition.=0A= + */=0A= + if (partconstr)=0A= + {=0A= + partconstr =3D (List *)=0A= + expression_planner((Expr *) partconstr);=0A= + if (rel->relid !=3D 1)=0A= + ChangeVarNodes((Node *) partconstr, 1,=0A= + rel->relid, 0);=0A= + if (predicate_refuted_by(partconstr,=0A= + list_make1(clause),=0A= + false))=0A= + {=0A= + *contradictory =3D true;=0A= + return NIL;=0A= + }=0A= + }=0A= + =0A= + /*=0A= * Since we only allow strict operators, check for any=0A= * contradicting IS NULL.=0A= */=0A= ------=_NextPart_000_0090_01D4E95C.A99A1EE0 Content-Type: application/octet-stream; name="v3_default_partition_pruning.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="v3_default_partition_pruning.patch" diff --git a/src/backend/partitioning/partprune.c = b/src/backend/partitioning/partprune.c=0A= index 59f34f5..5cd740a 100644=0A= --- a/src/backend/partitioning/partprune.c=0A= +++ b/src/backend/partitioning/partprune.c=0A= @@ -647,6 +647,7 @@ get_matching_partitions(PartitionPruneContext = *context, List *pruning_steps)=0A= PruneStepResult **results,=0A= *final_result;=0A= ListCell *lc;=0A= + bool scan_default;=0A= =0A= /* If there are no pruning steps then all partitions match. */=0A= if (num_steps =3D=3D 0)=0A= @@ -698,20 +699,23 @@ get_matching_partitions(PartitionPruneContext = *context, List *pruning_steps)=0A= Assert(final_result !=3D NULL);=0A= i =3D -1;=0A= result =3D NULL;=0A= + scan_default =3D final_result->scan_default;=0A= while ((i =3D bms_next_member(final_result->bound_offsets, i)) >=3D 0)=0A= {=0A= int partindex =3D context->boundinfo->indexes[i];=0A= =0A= /*=0A= - * In range and hash partitioning cases, some slots may contain -1,=0A= + * In range and hash partitioning cases, some index values may be -1,=0A= * indicating that no partition has been defined to accept a given=0A= - * range of data or for a given remainder, respectively. The default=0A= - * partition, if any, in case of range partitioning, will be added to=0A= - * the result, because the specified range still satisfies the query's=0A= - * conditions.=0A= + * range of values (that is, the bound at given offset is the upper=0A= + * bound of this unassigned range of values) or for a given remainder,=0A= + * respectively. As it's still part of the queried range of values,=0A= + * add the default partition, if any.=0A= */=0A= if (partindex >=3D 0)=0A= result =3D bms_add_member(result, partindex);=0A= + else=0A= + scan_default |=3D partition_bound_has_default(context->boundinfo);=0A= }=0A= =0A= /* Add the null and/or default partition if needed and if present. */=0A= @@ -721,7 +725,7 @@ get_matching_partitions(PartitionPruneContext = *context, List *pruning_steps)=0A= Assert(partition_bound_accepts_nulls(context->boundinfo));=0A= result =3D bms_add_member(result, context->boundinfo->null_index);=0A= }=0A= - if (final_result->scan_default)=0A= + if (scan_default)=0A= {=0A= Assert(context->strategy =3D=3D PARTITION_STRATEGY_LIST ||=0A= context->strategy =3D=3D PARTITION_STRATEGY_RANGE);=0A= @@ -2236,6 +2240,11 @@ get_matching_hash_bounds(PartitionPruneContext = *context,=0A= * get_matching_list_bounds=0A= * Determine the offsets of list bounds matching the specified value,=0A= * according to the semantics of the given operator strategy=0A= + *=0A= + * scan_default will be set in the returned struct, if the default = partition=0A= + * needs to be scanned, provided one exists at all. scan_null will be = set if=0A= + * the special null-accepting partition needs to be scanned.=0A= + *=0A= * 'opstrategy' if non-zero must be a btree strategy number.=0A= *=0A= * 'value' contains the value to use for pruning.=0A= @@ -2438,8 +2447,17 @@ get_matching_list_bounds(PartitionPruneContext = *context,=0A= * Each datum whose offset is in result is to be treated as the upper = bound of=0A= * the partition that will contain the desired values.=0A= *=0A= - * If default partition needs to be scanned for given values, set = scan_default=0A= - * in result if present.=0A= + * scan_default will be set in the returned struct, if the default = partition=0A= + * needs to be scanned, provided one exists at all. Although note that = we=0A= + * intentionally don't set scan_default in this function if only = because the=0A= + * matching set of values, found by comparing the input values using the=0A= + * specified opstrategy, contains unassigned portions of key space, = which=0A= + * we normally assume to belong to the default partition. We don't = infer=0A= + * that until all steps have been executed, including any combination = steps,=0A= + * because even if such unassinged portion of key space appears to be = part of=0A= + * the set of queried values based on comparisons in the individual = OpExprs,=0A= + * the final set of bounds obtained after combining multiple OpExprs may=0A= + * exclude it.=0A= *=0A= * 'opstrategy' if non-zero must be a btree strategy number.=0A= *=0A= @@ -2465,8 +2483,7 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= int *partindices =3D boundinfo->indexes;=0A= int off,=0A= minoff,=0A= - maxoff,=0A= - i;=0A= + maxoff;=0A= bool is_equal;=0A= bool inclusive =3D false;=0A= =0A= @@ -2530,11 +2547,7 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= if (nvalues =3D=3D partnatts)=0A= {=0A= /* There can only be zero or one matching partition. */=0A= - if (partindices[off + 1] >=3D 0)=0A= - result->bound_offsets =3D bms_make_singleton(off + 1);=0A= - else=0A= - result->scan_default =3D=0A= - partition_bound_has_default(boundinfo);=0A= + result->bound_offsets =3D bms_make_singleton(off + 1);=0A= return result;=0A= }=0A= else=0A= @@ -2622,57 +2635,21 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= maxoff =3D off + 1;=0A= }=0A= =0A= - /*=0A= - * Skip if minoff/maxoff are actually the upper bound of a=0A= - * un-assigned portion of values.=0A= - */=0A= - if (partindices[minoff] < 0 && minoff < boundinfo->ndatums)=0A= - minoff++;=0A= - if (partindices[maxoff] < 0 && maxoff >=3D 1)=0A= - maxoff--;=0A= -=0A= - /*=0A= - * There may exist a range of values unassigned to any=0A= - * non-default partition between the datums at minoff and=0A= - * maxoff. Add the default partition in that case.=0A= - */=0A= - if (partition_bound_has_default(boundinfo))=0A= - {=0A= - for (i =3D minoff; i <=3D maxoff; i++)=0A= - {=0A= - if (partindices[i] < 0)=0A= - {=0A= - result->scan_default =3D true;=0A= - break;=0A= - }=0A= - }=0A= - }=0A= -=0A= Assert(minoff >=3D 0 && maxoff >=3D 0);=0A= result->bound_offsets =3D bms_add_range(NULL, minoff, maxoff);=0A= }=0A= - else if (off >=3D 0) /* !is_equal */=0A= + else=0A= {=0A= /*=0A= * The lookup value falls in the range between some bounds in=0A= * boundinfo. 'off' would be the offset of the greatest bound=0A= * that is <=3D lookup value, so add off + 1 to the result=0A= * instead as the offset of the upper bound of the only=0A= - * partition that may contain the lookup value.=0A= + * partition that may contain the lookup value. If 'off' is=0A= + * -1 indicating that all bounds are greater, then we simply=0A= + * end up adding the first bound's offset, that is, 0.=0A= */=0A= - if (partindices[off + 1] >=3D 0)=0A= - result->bound_offsets =3D bms_make_singleton(off + 1);=0A= - else=0A= - result->scan_default =3D=0A= - partition_bound_has_default(boundinfo);=0A= - }=0A= - else=0A= - {=0A= - /*=0A= - * off < 0: the lookup value is smaller than all bounds, so=0A= - * only the default partition qualifies, if there is one.=0A= - */=0A= - result->scan_default =3D partition_bound_has_default(boundinfo);=0A= + result->bound_offsets =3D bms_make_singleton(off + 1);=0A= }=0A= =0A= return result;=0A= @@ -2770,16 +2747,7 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= boundinfo,=0A= nvalues, values,=0A= &is_equal);=0A= - if (off < 0)=0A= - {=0A= - /*=0A= - * All bounds are greater than the key, so we could only=0A= - * expect to find the lookup key in the default partition.=0A= - */=0A= - result->scan_default =3D partition_bound_has_default(boundinfo);=0A= - return result;=0A= - }=0A= - else=0A= + if (off >=3D 0)=0A= {=0A= /*=0A= * See the comment above.=0A= @@ -2827,6 +2795,12 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= else=0A= maxoff =3D off;=0A= }=0A= + else=0A= + /*=0A= + * 'off' is -1 indicating that all bounds are greater, so just=0A= + * set the first bound's offset as maxoff.=0A= + */=0A= + maxoff =3D off + 1;=0A= break;=0A= =0A= default:=0A= @@ -2834,58 +2808,43 @@ get_matching_range_bounds(PartitionPruneContext = *context,=0A= break;=0A= }=0A= =0A= + Assert(minoff >=3D 0 && minoff <=3D boundinfo->ndatums);=0A= + Assert(maxoff >=3D 0 && maxoff <=3D boundinfo->ndatums);=0A= +=0A= /*=0A= - * Skip a gap and when doing so, check if the bound contains a finite=0A= - * value to decide if we need to add the default partition. If it's an=0A= - * infinite bound, we need not add the default partition, as having an=0A= - * infinite bound means the partition in question catches any values = that=0A= - * would otherwise be in the default partition.=0A= + * If the smallest partition to return has MINVALUE (negative = infinity) as=0A= + * its lower bound, increment it to point to the next finite bound=0A= + * (supposedly its upper bound), so that we don't advertently end up=0A= + * scanning the default partition.=0A= */=0A= - if (partindices[minoff] < 0)=0A= + if (minoff < boundinfo->ndatums && partindices[minoff] < 0)=0A= {=0A= int lastkey =3D nvalues - 1;=0A= =0A= - if (minoff >=3D 0 &&=0A= - minoff < boundinfo->ndatums &&=0A= - boundinfo->kind[minoff][lastkey] =3D=3D=0A= - PARTITION_RANGE_DATUM_VALUE)=0A= - result->scan_default =3D partition_bound_has_default(boundinfo);=0A= -=0A= - minoff++;=0A= + if (boundinfo->kind[minoff][lastkey] =3D=3D=0A= + PARTITION_RANGE_DATUM_MINVALUE)=0A= + {=0A= + minoff++;=0A= + Assert(boundinfo->indexes[minoff] >=3D 0);=0A= + }=0A= }=0A= =0A= /*=0A= - * Skip a gap. See the above comment about how we decide whether or = not=0A= - * to scan the default partition based whether the datum that will = become=0A= - * the maximum datum is finite or not.=0A= + * If the previous greatest partition has MAXVALUE (positive infinity) = as=0A= + * its upper bound (something only possible to do with multi-column = range=0A= + * partitioning), we scan switch to it as the greatest partition to=0A= + * return. Again, so that we don't advertently end up scanning the=0A= + * default partition.=0A= */=0A= if (maxoff >=3D 1 && partindices[maxoff] < 0)=0A= {=0A= int lastkey =3D nvalues - 1;=0A= =0A= - if (maxoff >=3D 0 &&=0A= - maxoff <=3D boundinfo->ndatums &&=0A= - boundinfo->kind[maxoff - 1][lastkey] =3D=3D=0A= - PARTITION_RANGE_DATUM_VALUE)=0A= - result->scan_default =3D partition_bound_has_default(boundinfo);=0A= -=0A= - maxoff--;=0A= - }=0A= -=0A= - if (partition_bound_has_default(boundinfo))=0A= - {=0A= - /*=0A= - * There may exist a range of values unassigned to any non-default=0A= - * partition between the datums at minoff and maxoff. Add the default=0A= - * partition in that case.=0A= - */=0A= - for (i =3D minoff; i <=3D maxoff; i++)=0A= + if (boundinfo->kind[maxoff - 1][lastkey] =3D=3D=0A= + PARTITION_RANGE_DATUM_MAXVALUE)=0A= {=0A= - if (partindices[i] < 0)=0A= - {=0A= - result->scan_default =3D true;=0A= - break;=0A= - }=0A= + maxoff--;=0A= + Assert(boundinfo->indexes[maxoff] >=3D 0);=0A= }=0A= }=0A= =0A= @@ -3168,14 +3127,23 @@ = perform_pruning_combine_step(PartitionPruneContext *context,=0A= =0A= /*=0A= * A combine step without any source steps is an indication to not = perform=0A= - * any partition pruning, we just return all partitions.=0A= + * any partition pruning, we just return all datum offsets.=0A= */=0A= result =3D (PruneStepResult *) palloc0(sizeof(PruneStepResult));=0A= if (list_length(cstep->source_stepids) =3D=3D 0)=0A= {=0A= PartitionBoundInfo boundinfo =3D context->boundinfo;=0A= =0A= - result->bound_offsets =3D bms_add_range(NULL, 0, boundinfo->ndatums - = 1);=0A= + /*=0A= + * Add all valid offsets into the boundinfo->indexes array. For range=0A= + * partitioning, boundinfo->indexes contains (boundinfo->ndatums + 1)=0A= + * valid entries.=0A= + */=0A= + if (context->strategy =3D=3D PARTITION_STRATEGY_RANGE)=0A= + result->bound_offsets =3D bms_add_range(NULL, 0, boundinfo->ndatums);=0A= + else=0A= + result->bound_offsets =3D bms_add_range(NULL, 0,=0A= + boundinfo->ndatums - 1);=0A= result->scan_default =3D partition_bound_has_default(boundinfo);=0A= result->scan_null =3D partition_bound_accepts_nulls(boundinfo);=0A= return result;=0A= diff --git a/src/test/regress/expected/partition_prune.out = b/src/test/regress/expected/partition_prune.out=0A= index 7806ba1..263a1e5 100644=0A= --- a/src/test/regress/expected/partition_prune.out=0A= +++ b/src/test/regress/expected/partition_prune.out=0A= @@ -504,15 +504,13 @@ explain (costs off) select * from rlp where a <=3D = 31;=0A= Filter: (a <=3D 31)=0A= -> Seq Scan on rlp5_1=0A= Filter: (a <=3D 31)=0A= - -> Seq Scan on rlp5_default=0A= - Filter: (a <=3D 31)=0A= -> Seq Scan on rlp_default_10=0A= Filter: (a <=3D 31)=0A= -> Seq Scan on rlp_default_30=0A= Filter: (a <=3D 31)=0A= -> Seq Scan on rlp_default_default=0A= Filter: (a <=3D 31)=0A= -(29 rows)=0A= +(27 rows)=0A= =0A= explain (costs off) select * from rlp where a =3D 1 or a =3D 7;=0A= QUERY PLAN =0A= @@ -559,11 +557,7 @@ explain (costs off) select * from rlp where a > 20 = and a < 27;=0A= Filter: ((a > 20) AND (a < 27))=0A= -> Seq Scan on rlp4_2=0A= Filter: ((a > 20) AND (a < 27))=0A= - -> Seq Scan on rlp4_default=0A= - Filter: ((a > 20) AND (a < 27))=0A= - -> Seq Scan on rlp_default_default=0A= - Filter: ((a > 20) AND (a < 27))=0A= -(9 rows)=0A= +(5 rows)=0A= =0A= explain (costs off) select * from rlp where a =3D 29;=0A= QUERY PLAN =0A= ------=_NextPart_000_0090_01D4E95C.A99A1EE0--