| From: | Dongpo Liu <poe(dot)liu(at)pm(dot)me> |
|---|---|
| To: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com> |
| Subject: | pg_plan_advice fails on a CustomScan that replaces a join |
| Date: | 2026-10-05 08:22:52 |
| Message-ID: | sSMpyoJ12CV5Y3uS-T_8Z3M1xsJBELBd-Vh5DmlC9_jy9TOqMuWzr8-TuXrBpL8AlPlyEFkyQ4wX5guJCyVizY-R2UET0DpIEeA1aSsUjk4=@pm.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
This is about finding #19 in the defect report attached to Noah's
review of pg_plan_advice [1]: pgpa_relids() ignores
CustomScan.custom_relids. Robert wrote [2]:
> #19 might be a real bug, but needs validation, and can't be easily
> validated with in-core code.
I validated #19 with a small test-only custom scan provider (cjoin.c,
attached) and a script that uses it (cjoin.sql). The provider uses
set_join_pathlist_hook to replace the inner join of two tables named
cj_a and cj_b with one CustomScan that has scanrelid = 0. It only
builds the plan and cannot execute the join.
On master (852fd5b86e1), with cjoin loaded:
EXPLAIN (COSTS OFF, PLAN_ADVICE)
SELECT * FROM cj_a a JOIN cj_b b ON a.id = b.id
JOIN cj_c c ON c.id = a.id;
ERROR: XX000: plan node has no RTIs: 360
LOCATION: pgpa_build_scan, pgpa_scan.c:215
360 is T_CustomScan. The plan, from EXPLAIN without PLAN_ADVICE, is:
Hash Join
Hash Cond: (a.id = c.id)
-> Custom Scan (CustomJoin)
-> Hash
-> Seq Scan on cj_c c
When the custom scan is the topmost node, there is no error, but no
advice is generated for the two tables.
The same query without EXPLAIN also fails when
pg_plan_advice.always_store_advice_details is on, or when
pg_plan_advice.feedback_warnings is on and any advice is supplied.
The attached patch makes pgpa_relids() return custom_relids, in the
same way as fs_relids for a ForeignScan. With the patch, the query
above gives:
Generated Plan Advice:
JOIN_ORDER({a b} c)
HASH_JOIN(c)
SEQ_SCAN(c)
NO_GATHER(a b c)
This is the form that the README describes for a custom scan that
scans two tables.
With the patch, the pg_plan_advice, test_plan_advice and
test_extensible suites pass. The patch also applies to REL_19_STABLE,
but I tested it only on master.
The patch has no regression test, because a test needs a custom scan
provider that replaces a join. src/test/modules/test_extensible
(0e944fe3579) has a provider now, but only for a scan of one table.
Would it be acceptable to add a join mode to it for such a test? I
can write that.
[1] https://postgr.es/m/20260827171830.68.noahmisch@microsoft.com
[2] https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Best regards,
Dongpo Liu
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-pg_plan_advice-Handle-custom-scans-that-replace-a.patch | application/octet-stream | 1.8 KB |
| cjoin.c | text/plain | 5.1 KB |
| cjoin.sql | application/sql | 2.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Etsuro Fujita | 2026-10-05 08:22:54 | Re: postgres_fdw: transaction mode inheritance corner cases |
| Previous Message | Stefan Guha | 2026-10-05 08:10:52 | Re: Planning time quadratic in the IN-list length for "c = X AND (a, b) IN (...)" with BitmapOr |