public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tender Wang <[email protected]>
To: [email protected]
Cc: Robert Haas <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Ayush Tiwari <[email protected]>
Cc: Калинин Никита <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Pierre Forstmann <[email protected]>
Subject: Re: BUG #19493: Assertion failure in pg_plan_advice with EXISTS subquery and DO_NOT_SCAN advice
Date: Fri, 29 May 2026 10:03:50 +0800
Message-ID: <CAHewXNkVe1TQ1gi0_PzJhXw82AGK85Jc8+sLgC_CdtOzP_v+GA@mail.gmail.com> (raw)
In-Reply-To: <CAJTYsWUsW6zm_5thDsmV=c9VLs-O==r1RusDDF7kCekV0AECDA@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAHewXNn7q9Bm=i=ZBMJxUsmFhavfdxwbwVzdhsV9uXZOk584HQ@mail.gmail.com>
<CAHewXN=n_KzrOFgHyZwSHPuaXF-RRMig0o4yL+knoSE-_cMMPA@mail.gmail.com>
<CAHewXNkHsjOjaWUcdtrGTWeHK8f1N8=L434O0b9ecgtGaMFQrg@mail.gmail.com>
<CAJTYsWUsW6zm_5thDsmV=c9VLs-O==r1RusDDF7kCekV0AECDA@mail.gmail.com>
Hi all,
Ayush Tiwari <[email protected]> 于2026年5月27日周三 15:16写道：
>
> Hi,
>
> On Wed, 27 May 2026 at 09:20, Tender Wang <[email protected]> wrote:
>>
>> Hi, all
>>
>> I find an easier way as follows:
>> diff --git a/contrib/pg_plan_advice/pgpa_trove.c
>> b/contrib/pg_plan_advice/pgpa_trove.c
>> index ca69f3bd3df..64af4b1435b 100644
>> --- a/contrib/pg_plan_advice/pgpa_trove.c
>> +++ b/contrib/pg_plan_advice/pgpa_trove.c
>> @@ -179,7 +179,6 @@ pgpa_build_trove(List *advice_items)
>> * but in the future this
>> might not be true, e.g. a custom
>> * scan could replace a join.
>> */
>> - Assert(target->ttype ==
>> PGPA_TARGET_IDENTIFIER);
>> pgpa_trove_add_to_slice(&trove->scan,
>>
>> item->tag, target);
>> }
>
>
> Thanks for checking this.
>
> I agree that removing the assertion looks like the better approach.
I attached a patch to fix this issue.
In syntax.sql, I saw this:
"
-- Tags like SEQ_SCAN and NO_GATHER don't allow sublists at all; other tags,
-- except for JOIN_ORDER, allow at most one level of sublist. Hence, these
-- examples should error out.
"
So 'DO_NOT_SCAN((x))' is valid syntax. The original codes in
pgpa_build_trove() may
forget about this case. I added this syntax case to the syntax.sql.
I also added the query to scan.sql and adjusted the original comments.
--
Thanks,
Tender Wang
Attachments:
[application/octet-stream] 0001-pg_plan_advice-fix-assertion-failure-with-ordered-li.patch (5.3K, 2-0001-pg_plan_advice-fix-assertion-failure-with-ordered-li.patch)
download | inline diff:
From 122bf5295b4cc46200655d11ec17e4d8f6d72902 Mon Sep 17 00:00:00 2001
From: Tender Wang <[email protected]>
Date: Fri, 29 May 2026 09:37:46 +0800
Subject: [PATCH] pg_plan_advice: fix assertion failure with ordered-list scan
targets
Scan advice targets are not always represented as
PGPA_TARGET_IDENTIFIER. Some valid advice entries may use
PGPA_TARGET_ORDERED_LIST targets, causing the assertion in
pgpa_build_trove() to fail.
Remove the overly restrictive assertion and allow all valid
scan advice target types to be added to the scan trove.
Add a regression test covering ordered-list scan targets.
---
contrib/pg_plan_advice/expected/scan.out | 22 ++++++++++++++++++++++
contrib/pg_plan_advice/expected/syntax.out | 6 ++++++
contrib/pg_plan_advice/pgpa_trove.c | 9 +++++----
contrib/pg_plan_advice/sql/scan.sql | 7 +++++++
contrib/pg_plan_advice/sql/syntax.sql | 4 ++++
5 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/contrib/pg_plan_advice/expected/scan.out b/contrib/pg_plan_advice/expected/scan.out
index f4036e4cbdd..013ed1aa54e 100644
--- a/contrib/pg_plan_advice/expected/scan.out
+++ b/contrib/pg_plan_advice/expected/scan.out
@@ -769,6 +769,28 @@ SELECT * FROM (SELECT * FROM scan_table s WHERE a = 1 OFFSET 0);
NO_GATHER(unnamed_subquery s@unnamed_subquery)
(7 rows)
+COMMIT;
+-- Test sublist in Tags
+BEGIN;
+SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN((s1))';
+EXPLAIN(COSTS OFF, PLAN_ADVICE)
+SELECT * FROM scan_table s1 WHERE EXISTS (SELECT 1 FROM scan_table s2 WHERE s1.a = s2.a OFFSET 0);
+ QUERY PLAN
+----------------------------------------------------------------
+ Seq Scan on scan_table s1
+ Disabled: true
+ Filter: EXISTS(SubPlan exists_1)
+ SubPlan exists_1
+ -> Index Only Scan using scan_table_pkey on scan_table s2
+ Index Cond: (a = s1.a)
+ Supplied Plan Advice:
+ DO_NOT_SCAN((s1)) /* matched, failed */
+ Generated Plan Advice:
+ SEQ_SCAN(s1)
+ INDEX_ONLY_SCAN(s2@exists_1 public.scan_table_pkey)
+ NO_GATHER(s1 s2@exists_1)
+(12 rows)
+
COMMIT;
-- Test a non-repeatable tablesample method with a scan-level Materialize.
CREATE EXTENSION tsm_system_time;
diff --git a/contrib/pg_plan_advice/expected/syntax.out b/contrib/pg_plan_advice/expected/syntax.out
index c3f2cbd6dca..3366e544ce0 100644
--- a/contrib/pg_plan_advice/expected/syntax.out
+++ b/contrib/pg_plan_advice/expected/syntax.out
@@ -129,6 +129,12 @@ DETAIL: Could not parse advice: syntax error at or near "("
SET pg_plan_advice.advice = 'GATHER(((x)))';
ERROR: invalid value for parameter "pg_plan_advice.advice": "GATHER(((x)))"
DETAIL: Could not parse advice: syntax error at or near "("
+-- success
+SET pg_plan_advice.advice = 'DO_NOT_SCAN((x))';
+-- fail
+SET pg_plan_advice.advice = 'DO_NOT_SCAN(((x)))';
+ERROR: invalid value for parameter "pg_plan_advice.advice": "DO_NOT_SCAN(((x)))"
+DETAIL: Could not parse advice: syntax error at or near "("
-- Legal comments.
SET pg_plan_advice.advice = '/**/';
EXPLAIN (COSTS OFF) SELECT 1;
diff --git a/contrib/pg_plan_advice/pgpa_trove.c b/contrib/pg_plan_advice/pgpa_trove.c
index ca69f3bd3df..72c7fd025d3 100644
--- a/contrib/pg_plan_advice/pgpa_trove.c
+++ b/contrib/pg_plan_advice/pgpa_trove.c
@@ -175,11 +175,12 @@ pgpa_build_trove(List *advice_items)
foreach_ptr(pgpa_advice_target, target, item->targets)
{
/*
- * For now, all of our scan types target single relations,
- * but in the future this might not be true, e.g. a custom
- * scan could replace a join.
+ * Scan advice commonly targets a single relation, but this is not
+ * guaranteed by the representation: targets may also be ordered lists,
+ * and future scan advice might cover more complex targets such as a
+ * custom scan replacing a join. Therefore, do not assume identifier-only
+ * targets here.
*/
- Assert(target->ttype == PGPA_TARGET_IDENTIFIER);
pgpa_trove_add_to_slice(&trove->scan,
item->tag, target);
}
diff --git a/contrib/pg_plan_advice/sql/scan.sql b/contrib/pg_plan_advice/sql/scan.sql
index 98bee88de91..1440c93bc70 100644
--- a/contrib/pg_plan_advice/sql/scan.sql
+++ b/contrib/pg_plan_advice/sql/scan.sql
@@ -197,6 +197,13 @@ EXPLAIN (COSTS OFF, PLAN_ADVICE)
SELECT * FROM (SELECT * FROM scan_table s WHERE a = 1 OFFSET 0);
COMMIT;
+-- Test sublist in Tags
+BEGIN;
+SET LOCAL pg_plan_advice.advice = 'DO_NOT_SCAN((s1))';
+EXPLAIN(COSTS OFF, PLAN_ADVICE)
+SELECT * FROM scan_table s1 WHERE EXISTS (SELECT 1 FROM scan_table s2 WHERE s1.a = s2.a OFFSET 0);
+COMMIT;
+
-- Test a non-repeatable tablesample method with a scan-level Materialize.
CREATE EXTENSION tsm_system_time;
CREATE TABLE scan_tsm (i int);
diff --git a/contrib/pg_plan_advice/sql/syntax.sql b/contrib/pg_plan_advice/sql/syntax.sql
index f274fa48636..c9c5eb84f3a 100644
--- a/contrib/pg_plan_advice/sql/syntax.sql
+++ b/contrib/pg_plan_advice/sql/syntax.sql
@@ -42,6 +42,10 @@ SET pg_plan_advice.advice = '123';
-- examples should error out.
SET pg_plan_advice.advice = 'SEQ_SCAN((x))';
SET pg_plan_advice.advice = 'GATHER(((x)))';
+-- success
+SET pg_plan_advice.advice = 'DO_NOT_SCAN((x))';
+-- fail
+SET pg_plan_advice.advice = 'DO_NOT_SCAN(((x)))';
-- Legal comments.
SET pg_plan_advice.advice = '/**/';
--
2.34.1
view thread (11+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19493: Assertion failure in pg_plan_advice with EXISTS subquery and DO_NOT_SCAN advice
In-Reply-To: <CAHewXNkVe1TQ1gi0_PzJhXw82AGK85Jc8+sLgC_CdtOzP_v+GA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox