From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: allowing extensions to control planner behavior
In-reply-to: <CA+TgmoZY+baV-T-5ifDn6P=L=aV-VkVBrPmi0TQkcEq-5Finww@mail.gmail.com>
References: <CA+TgmoZY+baV-T-5ifDn6P=L=aV-VkVBrPmi0TQkcEq-5Finww@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
	message dated "Mon, 26 Aug 2024 12:32:53 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2909614.1724693854.1@sss.pgh.pa.us>
Date: Mon, 26 Aug 2024 13:37:34 -0400
Message-ID: <2909615.1724693854@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/2909615.1724693854%40sss.pgh.pa.us>
Precedence: bulk

Robert Haas <robertmhaas@gmail.com> writes:
> I'm somewhat expecting to be flamed to a well-done crisp for saying
> this, but I think we need better ways for extensions to control the
> behavior of PostgreSQL's query planner.

Nah, I won't flame you for that, it's a reasonable thing to think
about.  However, the devil is in the details, and ...

> The attached patch, briefly mentioned above, essentially converts the
> enable_* GUCs into RelOptInfo properties where the defaults are set by
> the corresponding GUCs.

... this doesn't seem like it's moving the football very far at all.
The enable_XXX GUCs are certainly blunt instruments, but I'm not sure
how much better it is if they're per-rel.  For example, I don't see
how this gets us any closer to letting an extension fix a poor choice
of join order.  Or, if your problem is that the planner wants to scan
index A but you want it to scan index B, enable_indexscan won't help.

> ... On the other hand, the more I look at
> what our enable_* GUCs actually do, the less impressed I am. IMHO,
> things like enable_hashjoin make a lot of sense, but enable_sort seems
> like it just controls an absolutely random smattering of behaviors in
> a way that seems to me to have very little to recommend it, and I've
> complained elsewhere about how enable_indexscan and
> enable_indexonlyscan are really quite odd when you look at how they're
> implemented.

Yeah, these sorts of questions aren't made better this way either.
If anything, having extensions manipulating these variables will
make it even harder to rethink what they do.

You mentioned that there is prior art out there, but this proposal
doesn't seem like it's drawing on any such thing.  What ideas should
we be stealing?

			regards, tom lane