From: "Jonathan Gonzalez V." <jonathan.abdiel@gmail.com>
To: Florin Irion <irionr@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org,  robertmhaas@gmail.com
Subject: Re: pg_plan_advice: add NO_ scan and join method tags
In-Reply-To: <123bbb65-ae76-4112-b33d-864aca745b7b@gmail.com> (Florin Irion's
	message of "Thu, 2 Jul 2026 18:35:24 +0200")
References: <123bbb65-ae76-4112-b33d-864aca745b7b@gmail.com>
Date: Tue, 14 Jul 2026 14:19:32 +0200
Message-ID: <87qzl5pxuz.fsf@gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
MIME-Version: 1.0
Content-Type: text/plain
Archived-At: 
 <https://www.postgresql.org/message-id/87qzl5pxuz.fsf%40gmail.com>
Precedence: bulk


Hello!

> Hello,
>
> The attached patch adds negative scan- and join-method advice to
> pg_plan_advice. These tags are the complement of the existing
> positive ones:

I was trying to measure the test coverage of this patch and I found that
the meson configuration wasn't updated, I think you're missing something
like this:

--- a/contrib/pg_plan_advice/meson.build
+++ b/contrib/pg_plan_advice/meson.build
@@ -57,6 +57,8 @@ tests += {
       'gather',
       'join_order',
       'join_strategy',
+      'no_join',
+      'no_scan',
       'partitionwise',
       'prepared',
       'scan',

After the tests run only two cases were not tested NO_MERGE_JOIN and
NO_NESTED_LOOP, but these supposed to be tested, well I'll dig more on
this one a second pass of the review.

During the check, this came to my attention:

+               case PGPA_TAG_NO_INDEX_ONLY_SCAN:
+                       return PGS_INDEXONLYSCAN | PGS_CONSIDER_INDEXONLY;

Reading the comments I got this:

 When PGS_CONSIDER_INDEXONLY is
 unset, we don't even consider index-only scans, and any such scans that
 would have been generated become index scans instead. On the other hand,
 unsetting PGS_INDEXSCAN or PGS_INDEXONLYSCAN causes generated paths of the
 corresponding types to be marked as disabled.

The message is a bit confusing but, I think that
`PGS_INDEXONLY` should be the only option here? Otherwise, my
understanding is that it will disable also PGS_INDEXSCAN, which is in
the next line to be disable.

Regards,
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com