| From: | Tender Wang <tndrwang(at)gmail(dot)com> |
|---|---|
| To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
| Subject: | Re: More partition pruning bugs with multi-column RANGE partitions |
| Date: | 2026-08-26 08:04:07 |
| Message-ID: | CAHewXNnBW5j0rCXCNKFzXzuXvS30Ocxvd0B19LVFmMgjMoeu8A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> 于2026年8月25日周二 22:24写道:
>
>
> I tested v1 and it fixes the reported wrong-result case.
>
> Using the same partition layout, I also tried:
>
> EXPLAIN (COSTS OFF) SELECT * FROM mc2ap WHERE a < 1;
>
> Master scans only mc2ap_def, while v1 scans both mc2ap1 and
> mc2ap_def. The result is unchanged, but it seems that the new check
> rejects nextoff == 0 even though that is a valid index into
> boundinfo->datums.
>
> Should the check use the actual array bounds instead?
>
> if (nextoff < 0 || nextoff >= boundinfo->ndatums)
>
> Would the same apply to the similar loop in the
> BTGreaterStrategyNumber case?
Hi,
I tested the similar loop in the BTGreaterStrategyNumber case and
confirmed that it has the same issue. Here is a small example:
```
CREATE TABLE r2 (a int, b int) PARTITION BY RANGE (a, b);
CREATE TABLE r2_p1 PARTITION OF r2
FOR VALUES FROM (1, 10) TO (1, 20);
CREATE TABLE r2_def PARTITION OF r2 DEFAULT;
EXPLAIN (COSTS OFF)
SELECT * FROM r2 WHERE a > 1;
```
With v1, both `r2_p1` and `r2_def` are scanned:
```
Append
-> Seq Scan on r2_p1 r2_1
Filter: (a > 1)
-> Seq Scan on r2_def r2_2
Filter: (a > 1)
```
`r2_p1` cannot contain any rows satisfying `a > 1`, so it should be
pruned. In this case, the two range bounds `(1, 10)` and `(1, 20)`
both compare equal to the lookup prefix `{1}`. Walking to
`nextoff == boundinfo->ndatums - 1` is therefore necessary to find
the appropriate edge of the matching bounds.
So I agree that the bounds check should be against the actual
`boundinfo->datums` array bounds, and that the same change is needed
for the similar loop in the BTGreaterStrategyNumber case.
I wrote the code to cover >= cases, as in the attached v2 (including
David's v1 code), and added the test case.
While looking at this, I also found the comment above the
BTLessStrategyNumber case a little misleading for prefix lookups:
```
/*
* Look for the greatest bound that is < or <= lookup value and
* set maxoff to its offset.
*/
```
When `nvalues < partnatts`, multiple range bounds can compare equal
to the lookup prefix, and `partition_range_datum_bsearch()` may
return one of those matching bounds rather than the greatest one.
The following loop then walks the adjacent bounds to find the
appropriate edge of that group.
Perhaps the comment could mention this distinction. For example,
something along the lines of:
```
/*
* Locate a bound at or below the lookup value. If the lookup
* contains only a prefix of the partition keys, multiple bounds
* may compare equal to it, so adjust off below to find the
* appropriate edge of the matching bounds.
*/
```
Similar wording might also be useful for the corresponding
BTGreaterStrategyNumber case.
The comment adjustments are not in the patch. I'm not sure there is a
need to adjust it.
--
Thanks,
Tender Wang
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Fix-incorrect-multi-column-RANGE-partition-prunin.patch | text/plain | 6.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nikhil Sontakke | 2026-08-26 08:07:08 | Re: [PATCH] Fix JSON_SERIALIZE() coercion placeholder type for jsonb input |
| Previous Message | Jakub Wartak | 2026-08-26 08:00:57 | Re: scary patch contest |