| From: | Tender Wang <tndrwang(at)gmail(dot)com> |
|---|---|
| To: | Chengpeng Yan <chengpeng_yan(at)outlook(dot)com> |
| Cc: | Jacob Brazeal <jacob(dot)brazeal(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Partition pruning can incorrectly exclude a RANGE default partition |
| Date: | 2026-07-28 02:36:10 |
| Message-ID: | CAHewXNk9ek5RkNY=U6p4OsXROG3PnFmd+WtKjyREpzTPQ_KAhw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Chengpeng Yan <chengpeng_yan(at)outlook(dot)com> 于2026年7月27日周一 22:53写道:
>
> Thanks for working on this. I may be missing something, but `off`
> indexes range-bound datums while `default_index` is a partition index,
> so I am not sure the two can be compared. With that patch I still get a
> wrong result:
>
> ```sql
> CREATE TABLE r (a int) PARTITION BY RANGE (a);
> CREATE TABLE r1 PARTITION OF r FOR VALUES FROM (0) TO (10);
> CREATE TABLE r2 PARTITION OF r FOR VALUES FROM (20) TO (30);
> CREATE TABLE rd PARTITION OF r DEFAULT;
> INSERT INTO r VALUES (5), (35);
>
> SELECT count(*) FROM r WHERE a IS NOT NULL AND a IN (5, 35);
> ```
>
> Here `default_index` is 2 but `off` is 3 for 35, so `scan_default` stays
> false, `rd` is pruned and the query returns 1 instead of 2.
>
Yes, I misunderstood the meaning of 'off'. When I saw the code in
create_range_bounds():
if (spec->is_default)
{
default_index = i;
continue;
}
I mistakenly thought that off and default_index were related in some way.
> I agree with Jacob that the problem is in the `nvalues == 0` path. For
> 35, the `IN` step keeps a `-1` offset, but the `IS NOT NULL` step
> removes that same offset. Their intersection therefore loses 35 and
> prunes the default partition. If the `IS NOT NULL` step keeps all
> offsets, `get_matching_partitions()` can later turn the surviving `-1`
> offset into a default-partition scan:
>
> ```diff
> diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
> index e7c318bbcac..48d16ebb4fb 100644
> --- a/src/backend/partitioning/partprune.c
> +++ b/src/backend/partitioning/partprune.c
> @@ -3031,15 +3031,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
> */
> if (nvalues == 0)
> {
> - /* ignore key space not covered by any partitions */
> - if (partindices[minoff] < 0)
> - minoff++;
> - if (partindices[maxoff] < 0)
> - maxoff--;
> -
> result->scan_default = partition_bound_has_default(boundinfo);
> - Assert(partindices[minoff] >= 0 &&
> - partindices[maxoff] >= 0);
> result->bound_offsets = bms_add_range(NULL, minoff, maxoff);
> return result;
> ```
I looked back at how the default partition were added to the final result.
In the last of get_matching_partitions(), we have:
...
while ((i = bms_next_member(final_result->bound_offsets, i)) >= 0)
{
int partindex;
Assert(i < context->boundinfo->nindexes);
partindex = context->boundinfo->indexes[i];
if (partindex < 0)
{
/*
* In range partitioning cases, if a partition index is -1 it
* means that the bound at the offset is the upper bound for a
* range not covered by any partition (other than a possible
* default partition). In hash partitioning, the same means no
* partition has been defined for the corresponding remainder
* value.
*
* In either case, the value is still part of the queried range of
* values, so mark to scan the default partition if one exists.
*/
scan_default |= partition_bound_has_default(context->boundinfo);
continue;
}
...
The code above can retrieve the default partition.
Removing the code in "nvalues ==0" can cause the logic to enter "if
(partindex<0)" in get_matching_partitions().
But I still think that we should make the prune result of '15'
prune_step correct.
diff --git a/src/backend/partitioning/partprune.c
b/src/backend/partitioning/partprune.c
index e7c318bbcac..4641941f597 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -3170,6 +3170,8 @@ get_matching_range_bounds(PartitionPruneContext *context,
* end up adding the first bound's
offset, that is, 0.
*/
result->bound_offsets =
bms_make_singleton(off + 1);
+ if (partindices[off + 1] < 0)
+ result->scan_default |=
partition_bound_has_default(boundinfo);
}
The above diff can do the same work as the code at the end of
get_matching_partitions().
--
Thanks,
Tender Wang
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-07-28 02:49:58 | Re: NULL pointer dereference in syslogger with load_libraries() and -DEXEC_BACKEND at startup |
| Previous Message | Michael Paquier | 2026-07-28 02:24:48 | Re: aio tests failing on newer Linux kernels |