allowing extensions to control planner behavior
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. I know of two major reasons
why somebody might want to do this. First, you might want to do
something like what pg_hint_plan does, where it essentially implements
Oracle-style hints that can be either inline or stored in a side table
and automatically applied to queries.[1]https://github.com/ossc-db/pg_hint_plan In addition to supporting
Oracle-style hints, it also supports some other kinds of hints so that
you can, for example, try to fix broken cardinality estimates. Second,
you might want to convince the planner to keep producing the same kind
of plan that it produced previously. I believe this is what Amazon's
query plan management feature[2]https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/AuroraPostgreSQL.Optimize.html does, although since it is closed
source and I don't work at Amazon maybe it's actually implemented
completely differently. Regardless of what Amazon did in this case,
plan stability is a feature people want. Just trying to keep using the
same plan data structure forever doesn't seem like a good strategy,
because for example it would be fragile in the case of any DDL
changes, like dropping and recreating an index, or dropping or adding
a column. But you might want conceptually the same plan. Although it's
not frequently admitted on this mailing list, unexpected plan changes
are a frequent cause of sudden database outages, and wanting to
prevent that is a legitimate thing for a user to try to do. Naturally,
there is a risk that you might in so doing also prevent plan changes
that would have dramatically improved performance, or stick with a
plan long after you've outgrown it, but that doesn't stop people from
wanting it, or other databases (or proprietary forks of this database)
from offering it, and I don't think it should.
We have some hooks right now that offer a few options in this area,
but there are problems. The hook that I believe to be closest to the
right thing is this one:
/*
* Allow a plugin to editorialize on the set of Paths for this base
* relation. It could add new paths (such as CustomPaths) by calling
* add_path(), or add_partial_path() if parallel aware. It could also
* delete or modify paths added by the core code.
*/
if (set_rel_pathlist_hook)
(*set_rel_pathlist_hook) (root, rel, rti, rte);
Unfortunately, the part about the hook having the freedom to delete
paths isn't really true. Perhaps technically you can delete a path
that you don't want to be chosen, but any paths that were dominated by
the path you deleted have already been thrown away and it's too late
to get them back. You can modify paths if you don't want to change
their costs, but if you change their costs then you have the same
problem: the contents of the pathlist at the time that you see it are
determined by the costs that each path had when it was initially
added, and it's really too late to editorialize on that. So all you
can really do here in practice is add new paths.
set_join_pathlist_hook, which applies to joinrels, is similarly
limited. appendrels don't even have an equivalent of this hook.
So, how could we do better?
I think there are two basic approaches that are possible here. If
someone sees a third option, let me know. First, we could allow users
to hook add_path() and add_partial_path(). That certainly provides the
flexibility on paper to accept or reject whatever paths you do or do
not want. However, I don't find this approach very appealing. One
problem is that it's likely to be fairly expensive, because add_path()
gets called A LOT. A second problem is that you don't actually get an
awful lot of context: I think anybody writing a hook would have to
write code to basically analyze each proposed path and figure out why
it was getting added and then decide what to do. In some cases that
might be fine, because for example accepting or rejecting paths based
on path type seems fairly straightforward with this approach, but as
soon as you want to do anything more complicated than that it starts
to seem difficult. If, for example, you want relation R1 to be the
driving table for the whole query plan, you're going to have to
determine whether or not that is the case for every single candidate
(partial) path that someone hands you, so you're going to end up
making that decision a whole lot of times. It doesn't sound
particularly fun. Third, even if you are doing something really simple
like trying to reject mergejoins, you've already lost the opportunity
to skip a bunch of work. If you had known when you started planning
the joinrel that you didn't care about mergejoins, you could have
skipped looking for merge-joinable clauses. Overall, while I wouldn't
be completely against further exploration of this option, I suspect
it's pretty hard to do anything useful with it.
The other possible approach is to allow extensions to feed some
information into the planner before path generation and let that
influence which paths are generated. This is essentially what
pg_hint_plan is doing: it implements plan type hints by arranging to
flip the various enable_* GUCs on and off during the planning of
various rels. That's clever but ugly, and it ends up duplicating
substantial chunks of planner code due to the inadequacy of the
existing hooks. With some refactoring and some additional hooks, we
could make this much less ugly. But that gets at what I believe to be
the core difficulty of this approach, which is that the core planner
code needs to be somewhat aware of and on board with what the user or
the extension is trying to do. If an extension wants to force the join
order, that is likely to require different scaffolding than if it
wants to force the join methods which is again different from if a
hook wants to bias the query planner towards or against particular
indexes. Putting in hooks or other infrastructure that allows an
extension to control a particular aspect of planner behavior is to
some extent an endorsement of controlling the planner behavior in that
particular way. Since any amount of allowing the user to control the
planner tends to be controversial around here, that opens up the
spectre of putting a whole lot of effort into arguing about which
things extensions should be allowed to do, getting most of the patches
rejected, and ending up with nothing that's actually useful.
But on the other hand, it's not like we have to design everything in a
greenfield. Other database systems have provided in-core, user-facing
features to control the planner for decades, and we can look at those
offerings -- and existing offerings in the PG space -- as we try to
judge whether a particular use case is totally insane. I am not here
to argue that everything that every system has done is completely
perfect and without notable flaws, but our own system has its own
share of flaws, and the fact that you can do very little when a
previously unproblematic query starts suddenly producing a bad plan is
definitely one of them. I believe we are long past the point where we
can simply hold our breath and pretend like there's no issue here. At
the very least, allowing extensions to control scan methods (including
choice of indexes), join methods, and join order (including which
table ends up on which side of a given join) and similar things for
aggregates and appendrels seems to me like it ought to be table
stakes. And those extensions shouldn't have to duplicate large chunks
of code or resort to gross hacks to do it. Eventually, maybe we'll
even want to have directly user-facing features to do some of this
stuff (in query hints, out of query hints, or whatever) but I think
opening the door up to extensions doing it is a good first step,
because (1) that allows different extensions to do different things
without taking a position on what the One Right Thing To Do is and (2)
if it becomes clear that something improvident has been done, it is a
lot easier to back out a hook or some C API change than it is to
back-out a user-visible feature. Or maybe we'll never want to expose a
user-visible feature here, but it can still be useful to enable
extensions.
The attached patch, briefly mentioned above, essentially converts the
enable_* GUCs into RelOptInfo properties where the defaults are set by
the corresponding GUCs. The idea is that a hook could then change this
on a per-RelOptInfo basis before path generation happens. For
baserels, I believe that could be done from get_relation_info_hook for
baserels, and we could introduce something similar for other kinds of
rels. I don't think this is in any way the perfect approach. On the
one hand, it doesn't give you all the kinds of control over path
generation that you might want. 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. Still, this seemed like a fairly easy thing to do as a
way of demonstrating the kind of thing that we could do to provide
extensions with more control over planner behavior, and I believe it
would be concretely useful to pg_hint_plan in particular. But all that
said, as much as anything, I want to get some feedback on what
approaches and trade-offs people think might be acceptable here,
because there's not much point in me spending a bunch of time writing
code that everyone (or a critical mass of people) are going to hate.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
[1]: https://github.com/ossc-db/pg_hint_plan
[2]: https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/AuroraPostgreSQL.Optimize.html
Attachments:
v1-0001-Convert-enable_-GUCs-into-per-RelOptInfo-values-w.patchapplication/octet-stream; name=v1-0001-Convert-enable_-GUCs-into-per-RelOptInfo-values-w.patchDownload+288-149
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
On 26/8/2024 18:32, Robert Haas wrote:
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. I know of two major reasons
It is the change I have been waiting for a long time. Remember how many
kludge codes in pg_hint_plan, aqo, citus, timescale, etc., are written
for only the reason of a small number of hooks - I guess many other
people could cheer such work.
why somebody might want to do this. First, you might want to do
something like what pg_hint_plan does, where it essentially implements
Oracle-style hints that can be either inline or stored in a side table
and automatically applied to queries.[1] In addition to supporting
Oracle-style hints, it also supports some other kinds of hints so that
you can, for example, try to fix broken cardinality estimates. Second,
My personal most wanted list:
- Selectivity list estimation hook
- Groups number estimation hook
- hooks on memory estimations, involving work_mem
- add_path() hook
- Hook on final RelOptInfo pathlist
- a custom list of nodes in RelOptinfo, PlannerStmt, Plan and Query
structures
- Extensibility of extended and plain statistics
- Hook on portal error processing
- Canonicalise expressions hook
you might want to convince the planner to keep producing the same kind
of plan that it produced previously. I believe this is what Amazon's
query plan management feature[2] does, although since it is closed
source and I don't work at Amazon maybe it's actually implemented
completely differently. Regardless of what Amazon did in this case,
plan stability is a feature people want. Just trying to keep using the
same plan data structure forever doesn't seem like a good strategy,
because for example it would be fragile in the case of any DDL
changes, like dropping and recreating an index, or dropping or adding
As a designer of plan freezing feature [1]https://postgrespro.com/docs/enterprise/16/sr-plan I can say it utilises
plancache and, being under its invalidation callbacks it doesn't afraid
DDL or any other stuff altering database objects.
Unfortunately, the part about the hook having the freedom to delete
paths isn't really true. Perhaps technically you can delete a path
that you don't want to be chosen, but any paths that were dominated by
the path you deleted have already been thrown away and it's too late
to get them back. You can modify paths if you don't want to change
their costs, but if you change their costs then you have the same
problem: the contents of the pathlist at the time that you see it are
determined by the costs that each path had when it was initially
added, and it's really too late to editorialize on that. So all you
can really do here in practice is add new paths.
From my standpoint, it is enough to export routines creating paths and
calculating costs.
set_join_pathlist_hook, which applies to joinrels, is similarly
limited. appendrels don't even have an equivalent of this hook.So, how could we do better?
I think there are two basic approaches that are possible here. If
someone sees a third option, let me know. First, we could allow users
to hook add_path() and add_partial_path(). That certainly provides the
flexibility on paper to accept or reject whatever paths you do or do
+1
The attached patch, briefly mentioned above, essentially converts the
enable_* GUCs into RelOptInfo properties where the defaults are set by
the corresponding GUCs. The idea is that a hook could then change this
on a per-RelOptInfo basis before path generation happens. For
IMO, it is better not to switch on/off algorithms, but allow extensions
to change their cost multipliers, modifying costs balance. 10E9 looks
like a disable, but multiplier == 10 for a cost node just provide more
freedom for hashing strategies.
[1]: https://postgrespro.com/docs/enterprise/16/sr-plan
--
regards, Andrei Lepikhov
On Mon, Aug 26, 2024 at 1:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
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 ...
Thank you. Not being flamed is one of my favorite things. :-)
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.
Well, I agree that this doesn't address everything you might want to
do, and I thought I said so, admittedly in the middle of a long wall
of text. This would JUST be a step toward letting an extension control
the scan and join methods, not the join order or the choice of index
or whatever else there is. But the fact that it doesn't do everything
is not a strike against it unless there's some competing design that
lets you take care of everything with a single mechanism, which I do
not see as realistic. If this proposal -- or really any proposal in
this area -- gets through, I will very happily propose more things to
address the other problems that I know about, but it doesn't make
sense to do a huge amount of work to craft a comprehensive solution
before we've had any discussion here.
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.
Correct, but my proposal to make enable_indexscan behave like
enable_indexonlyscan, which I thought was a slam-dunk, just provoked a
lot of grumbling. There's a kind of chicken and egg problem here. If
the existing GUCs were better designed, then using them here would
make sense. And the patch that I attached to my previous email were in
master, then cleaning up the design of the GUCs would have more value.
But if I can't make any progress with either problem because the other
problem also exists, then I'm kind of boxed into a corner. I could
also propose something here that is diverges from the enable_*
behavior, but then people will complain that the two shouldn't be
inconsistent, which I agree with, BTW. I thought maybe doing this
first would make sense, and then we could refine afterwards.
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?
Depends what you mean. As far as PostgreSQL-related things, the two
things that I mentioned in my opening paragraph and for which I
provided links seem to be me to the best examples we have. It's pretty
easy to see how to make pg_hint_plan require less kludgery, and I
think we can just iterate through the various problems there and solve
them pretty easily by adding a few hooks here and there and a few
extension-settable structure members here and there. I am engaging in
some serious hand-waving here, but this is not rocket science. I am
confident that if you made it your top priority to get into PG 18
stuff which would thoroughly un-hackify pg_hint_plan, you could be
done in months, possibly weeks. It will take me longer, but if we have
an agreement in principal that it is worth doing, I just can't see it
as being particularly difficult.
Amazon's query plan management stuff is a much tougher lift. For that,
you're asking the planner to try to create a new plan which is like
some old plan that you got before. So in a perfect world, you want to
control every planner decision. That's hard just because there are a
lot of them. If for example you want to get the same index scan that
you got before, you need not only to get the same type of index scan
(index, index-only, bitmap) and the same index, but also things like
the same non-native saop treatment, which seems like it would be
asking an awful lot of a hook system. On the other hand, maybe you can
cheat. If your regurgitate-the-same-plan system could force the same
join order, join methods, scan methods, choice of indexes, and
probably some stuff about aggregate and appendrel strategy, it might
be close enough to giving you the same plan you had before that nobody
would really care if the non-native saop treatment was different. I'm
almost positive it's better than not having a feature, which is where
are today. And although allowing control over just the major decisions
in query planning doesn't seem like something we can do in one patch,
I don't think it takes 100 patches either. Maybe five or ten.
If we step outside of the PostgreSQL ecosystem, I think we should look
at Oracle as one example. I have never been a real believer in hints
like SeqScan(foo), because if you don't fix the cardinality estimate
for table foo, then the rest of your plan is going to suck, too. On
the other hand, "hint everything" for some people in some situations
is a way to address that. It's stupid in a sense, but if you have an
automated way to do it, especially one that allows applying hints
out-of-query, it's not THAT stupid. Also, Oracle offers some other
pretty useful hints. In particular, using the LEADING hint to set the
driving table for the query plan does not seem dumb to me at all.
Hinting that things should be parallel or not, and with what degree of
parallelism, also seem quite reasonable. They've also got ALL_ROWS and
FIRST_ROWS(n) hints, which let you say whether you want fast-start
behavior or not, and it hardly needs to be said how often we get that
wrong or how badly. pg_hint_plan, which copies a lot of stuff that
Oracle does, innovates by allowing you to hint that a certain join
will return X number of rows or that the number or rows that the
planner thinks should be returned should be corrected by multiplying,
adding, or subtracting some constant. I'm not sure how useful this is
really because I feel like a lot of times you'd just pick some join
order where that particular join is no longer used e.g. if. A JOIN B
JOIN C and I hint the AB join, perhaps the planner will just start by
joining C to either A or B, and then that join will never occur.
However, that can be avoided by also using LEADING, or maybe in some
other cleverer way, like making an AB selectivity hint apply at
whatever point in the plan you join something that includes A to
something that includes B.
There's some details on SQL server's hinting here:
https://learn.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql?view=sql-server-ver16
It looks pretty complicated, but some of the basic concepts that you'd
expect are also present here: force the join method, rule in or out,
force the use of an index or of no index, force the join order. Those
seem to be the major things that "everyone" supports. I think we'd
want to expand a bit on that to allow forcing aggregate strategy and
perhaps some PostgreSQL-specific things e.g. other systems won't have
a hint to force a TIDRangeScan or not because that's a
PostgreSQL-specific concept, but it would be silly to make a system
that lets an extension control sequential scans and index scans but
not other, more rarely-used ways of scanning a relation, so probably
we want to do something.
I don't know if that helps, in terms of context. If it doesn't, let me
know what would help. And just to be clear, I *absolutely* think we
need to take a light touch here. If we install a ton of new
highly-opinionated infrastructure we will make a lot of people mad and
that's definitely not where I want to end up. I just think we need to
grow beyond "the planner is a black box and you shall not presume to
direct it." If every other system provides a way to control, say, the
join order, then it seems reasonable to suppose that a PostgreSQL
extension should be able to control the join order too. A lot of
details might be different but if multiple other systems have the
concept then the concept itself probably isn't ridiculous.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, Aug 26, 2024 at 2:00 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
It is the change I have been waiting for a long time. Remember how many
kludge codes in pg_hint_plan, aqo, citus, timescale, etc., are written
for only the reason of a small number of hooks - I guess many other
people could cheer such work.
I think so, too. I know there are going to be people who hate this,
but I think the cat is already out of the bag. It's not a question any
more of whether it will happen, it's just a question of whether we
want to collaborate with extension developers or try to make their
life difficult.
My personal most wanted list:
- Selectivity list estimation hook
- Groups number estimation hook
- hooks on memory estimations, involving work_mem
- add_path() hook
- Hook on final RelOptInfo pathlist
- a custom list of nodes in RelOptinfo, PlannerStmt, Plan and Query
structures
- Extensibility of extended and plain statistics
- Hook on portal error processing
- Canonicalise expressions hook
One of my chronic complaints about hooks is that people propose hooks
that are just in any random spot in the code where they happen to want
to change something. If we accept that, we end up with a lot of hooks
where nobody can say how the hook can be used usefully and maybe it
can't actually be used usefully even by the original author, or only
them and nobody else. So these kinds of proposals need detailed,
case-by-case scrutiny. It's unacceptable for the planner to get filled
up with a bunch of poorly-designed hooks just as it is for any other
part of the system, but well-designed hooks whose usefulness can
clearly be seen should be just as welcome here as anywhere else.
IMO, it is better not to switch on/off algorithms, but allow extensions
to change their cost multipliers, modifying costs balance. 10E9 looks
like a disable, but multiplier == 10 for a cost node just provide more
freedom for hashing strategies.
That may be a valid use case, but I do not think it is a typical use
case. In my experience, when people want to force the planner to do
something, they really mean it. They don't mean "please do it this way
unless you really, really don't feel like it." They mean "please do it
this way, period." And that is also what other systems provide. Oracle
could provide a hint MERGE_COST(foo,10) meaning make merge joins look
ten times as expensive but in fact they only provide MERGE and
NO_MERGE. And a "reproduce this previous plan" feature really demands
infrastructure that truly forces the planner to do what it's told,
rather than just nicely suggesting that it might want to do as it's
told. I wouldn't be sad at all if we happen to end up with a system
that's powerful enough for an extension to implement "make merge joins
ten times as expensive"; in fact, I think that would be pretty cool.
But I don't think it should be the design center for what we
implement, because it looks nothing like what existing PG or non-PG
systems do, at least in my experience.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 26/8/2024 21:44, Robert Haas wrote:
On Mon, Aug 26, 2024 at 2:00 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
My personal most wanted list:
- Selectivity list estimation hook
- Groups number estimation hook
- hooks on memory estimations, involving work_mem
- add_path() hook
- Hook on final RelOptInfo pathlist
- a custom list of nodes in RelOptinfo, PlannerStmt, Plan and Query
structures
- Extensibility of extended and plain statistics
- Hook on portal error processing
- Canonicalise expressions hookOne of my chronic complaints about hooks is that people propose hooks
that are just in any random spot in the code where they happen to want
to change something. If we accept that, we end up with a lot of hooks
where nobody can say how the hook can be used usefully and maybe it
can't actually be used usefully even by the original author, or only
them and nobody else. So these kinds of proposals need detailed,
case-by-case scrutiny. It's unacceptable for the planner to get filled
up with a bunch of poorly-designed hooks just as it is for any other
part of the system, but well-designed hooks whose usefulness can
clearly be seen should be just as welcome here as anywhere else.
Definitely so. Think about that as a sketch proposal on the roadmap.
Right now, I know about only one hook - selectivity hook - which we
already discussed and have Tomas Vondra's patch on the table. But even
this is a big deal, because multi-clause estimations are a huge pain for
users that can't be resolved with extensions for now without core patches.
IMO, it is better not to switch on/off algorithms, but allow extensions
to change their cost multipliers, modifying costs balance. 10E9 looks
like a disable, but multiplier == 10 for a cost node just provide more
freedom for hashing strategies.That may be a valid use case, but I do not think it is a typical use
case. In my experience, when people want to force the planner to do
something, they really mean it. They don't mean "please do it this way
unless you really, really don't feel like it." They mean "please do it
this way, period." And that is also what other systems provide. Oracle
could provide a hint MERGE_COST(foo,10) meaning make merge joins look
ten times as expensive but in fact they only provide MERGE and
NO_MERGE. And a "reproduce this previous plan" feature really demands
infrastructure that truly forces the planner to do what it's told,
rather than just nicely suggesting that it might want to do as it's
told. I wouldn't be sad at all if we happen to end up with a system
that's powerful enough for an extension to implement "make merge joins
ten times as expensive"; in fact, I think that would be pretty cool.
But I don't think it should be the design center for what we
implement, because it looks nothing like what existing PG or non-PG
systems do, at least in my experience.
Heh, I meant not manual usage, but automatical one, provided by extensions.
--
regards, Andrei Lepikhov
At 2024-08-27 00:32:53, "Robert Haas" <robertmhaas@gmail.com> wrote:
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. I know of two major reasons
why somebody might want to do this. First, you might want to do
something like what pg_hint_plan does, where it essentially implements
Oracle-style hints that can be either inline or stored in a side table
and automatically applied to queries.[1] In addition to supporting
Oracle-style hints, it also supports some other kinds of hints so that
you can, for example, try to fix broken cardinality estimates. Second,
you might want to convince the planner to keep producing the same kind
of plan that it produced previously. I believe this is what Amazon's
query plan management feature[2] does, although since it is closed
source and I don't work at Amazon maybe it's actually implemented
completely differently. Regardless of what Amazon did in this case,
plan stability is a feature people want. Just trying to keep using the
same plan data structure forever doesn't seem like a good strategy,
because for example it would be fragile in the case of any DDL
changes, like dropping and recreating an index, or dropping or adding
a column. But you might want conceptually the same plan. Although it's
not frequently admitted on this mailing list, unexpected plan changes
are a frequent cause of sudden database outages, and wanting to
prevent that is a legitimate thing for a user to try to do. Naturally,
there is a risk that you might in so doing also prevent plan changes
that would have dramatically improved performance, or stick with a
plan long after you've outgrown it, but that doesn't stop people from
wanting it, or other databases (or proprietary forks of this database)
from offering it, and I don't think it should.We have some hooks right now that offer a few options in this area,
but there are problems. The hook that I believe to be closest to the
right thing is this one:/*
* Allow a plugin to editorialize on the set of Paths for this base
* relation. It could add new paths (such as CustomPaths) by calling
* add_path(), or add_partial_path() if parallel aware. It could also
* delete or modify paths added by the core code.
*/
if (set_rel_pathlist_hook)
(*set_rel_pathlist_hook) (root, rel, rti, rte);Unfortunately, the part about the hook having the freedom to delete
paths isn't really true. Perhaps technically you can delete a path
that you don't want to be chosen, but any paths that were dominated by
the path you deleted have already been thrown away and it's too late
to get them back. You can modify paths if you don't want to change
their costs, but if you change their costs then you have the same
problem: the contents of the pathlist at the time that you see it are
determined by the costs that each path had when it was initially
added, and it's really too late to editorialize on that. So all you
can really do here in practice is add new paths.
set_join_pathlist_hook, which applies to joinrels, is similarly
limited. appendrels don't even have an equivalent of this hook.So, how could we do better?
I think there are two basic approaches that are possible here. If
someone sees a third option, let me know. First, we could allow users
to hook add_path() and add_partial_path(). That certainly provides the
flexibility on paper to accept or reject whatever paths you do or do
not want. However, I don't find this approach very appealing. One
problem is that it's likely to be fairly expensive, because add_path()
gets called A LOT. A second problem is that you don't actually get an
awful lot of context: I think anybody writing a hook would have to
write code to basically analyze each proposed path and figure out why
it was getting added and then decide what to do. In some cases that
might be fine, because for example accepting or rejecting paths based
on path type seems fairly straightforward with this approach, but as
soon as you want to do anything more complicated than that it starts
to seem difficult. If, for example, you want relation R1 to be the
driving table for the whole query plan, you're going to have to
determine whether or not that is the case for every single candidate
(partial) path that someone hands you, so you're going to end up
making that decision a whole lot of times. It doesn't sound
particularly fun. Third, even if you are doing something really simple
like trying to reject mergejoins, you've already lost the opportunity
to skip a bunch of work. If you had known when you started planning
the joinrel that you didn't care about mergejoins, you could have
skipped looking for merge-joinable clauses. Overall, while I wouldn't
be completely against further exploration of this option, I suspect
it's pretty hard to do anything useful with it.The other possible approach is to allow extensions to feed some
information into the planner before path generation and let that
influence which paths are generated. This is essentially what
pg_hint_plan is doing: it implements plan type hints by arranging to
flip the various enable_* GUCs on and off during the planning of
various rels. That's clever but ugly, and it ends up duplicating
substantial chunks of planner code due to the inadequacy of the
existing hooks. With some refactoring and some additional hooks, we
could make this much less ugly. But that gets at what I believe to be
the core difficulty of this approach, which is that the core planner
code needs to be somewhat aware of and on board with what the user or
the extension is trying to do. If an extension wants to force the join
order, that is likely to require different scaffolding than if it
wants to force the join methods which is again different from if a
hook wants to bias the query planner towards or against particular
indexes. Putting in hooks or other infrastructure that allows an
extension to control a particular aspect of planner behavior is to
some extent an endorsement of controlling the planner behavior in that
particular way. Since any amount of allowing the user to control the
planner tends to be controversial around here, that opens up the
spectre of putting a whole lot of effort into arguing about which
things extensions should be allowed to do, getting most of the patches
rejected, and ending up with nothing that's actually useful.But on the other hand, it's not like we have to design everything in a
greenfield. Other database systems have provided in-core, user-facing
features to control the planner for decades, and we can look at those
offerings -- and existing offerings in the PG space -- as we try to
judge whether a particular use case is totally insane. I am not here
to argue that everything that every system has done is completely
perfect and without notable flaws, but our own system has its own
share of flaws, and the fact that you can do very little when a
previously unproblematic query starts suddenly producing a bad plan is
definitely one of them. I believe we are long past the point where we
can simply hold our breath and pretend like there's no issue here. At
the very least, allowing extensions to control scan methods (including
choice of indexes), join methods, and join order (including which
table ends up on which side of a given join) and similar things for
aggregates and appendrels seems to me like it ought to be table
stakes. And those extensions shouldn't have to duplicate large chunks
of code or resort to gross hacks to do it. Eventually, maybe we'll
even want to have directly user-facing features to do some of this
stuff (in query hints, out of query hints, or whatever) but I think
opening the door up to extensions doing it is a good first step,
because (1) that allows different extensions to do different things
without taking a position on what the One Right Thing To Do is and (2)
if it becomes clear that something improvident has been done, it is a
lot easier to back out a hook or some C API change than it is to
back-out a user-visible feature. Or maybe we'll never want to expose a
user-visible feature here, but it can still be useful to enable
extensions.The attached patch, briefly mentioned above, essentially converts the
enable_* GUCs into RelOptInfo properties where the defaults are set by
the corresponding GUCs. The idea is that a hook could then change this
on a per-RelOptInfo basis before path generation happens. For
baserels, I believe that could be done from get_relation_info_hook for
baserels, and we could introduce something similar for other kinds of
rels. I don't think this is in any way the perfect approach. On the
one hand, it doesn't give you all the kinds of control over path
generation that you might want. 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. Still, this seemed like a fairly easy thing to do as a
way of demonstrating the kind of thing that we could do to provide
extensions with more control over planner behavior, and I believe it
would be concretely useful to pg_hint_plan in particular. But all that
said, as much as anything, I want to get some feedback on what
approaches and trade-offs people think might be acceptable here,
because there's not much point in me spending a bunch of time writing
code that everyone (or a critical mass of people) are going to hate.Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
[2] https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/AuroraPostgreSQL.Optimize.html
I really admire this idea.
here is my confusion: Isn't the core of this idea whether to turn the planner into a framework? Personally, I think that under PostgreSQL's heap table storage, the optimizer might be better off focusing on optimizing the generation of execution plans. It’s possible that in some specific scenarios, developers might want to intervene in the generation of execution plans by extensions. I'm not sure if these scenarios usually occur when the storage structure is also extended by developers. If so, could existing solutions like "planner_hook" potentially solve the problem?
On Mon, Aug 26, 2024 at 3:28 PM Robert Haas <robertmhaas@gmail.com> wrote:
Well, I agree that this doesn't address everything you might want to
do, ... I will very happily propose more things to
address the other problems that I know about ...
In that vein, here's a new patch set where I've added a second patch
that allows extensions to control choice of index. It's 3 lines of new
code, plus 7 lines of comments and whitespace. Feeling inspired, I
also included a contrib module, initial_vowels_are_evil, to
demonstrate how this can be used by an extension that wants to disable
certain indexes but not others. This is obviously quite silly and we
might (or might not) want a more serious example in contrib, but it
demonstrates how easy this can be with just a tiny bit of core
infrastructure:
robert.haas=# load 'initial_vowels_are_evil';
LOAD
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------
Aggregate (cost=2854.29..2854.30 rows=1 width=8)
-> Index Only Scan using pgbench_accounts_pkey on pgbench_accounts
(cost=0.29..2604.29 rows=100000 width=0)
(2 rows)
robert.haas=# alter index pgbench_accounts_pkey rename to
evil_pgbench_accounts_pkey;
ALTER INDEX
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
------------------------------------------------------------------------------
Aggregate (cost=2890.00..2890.01 rows=1 width=8)
-> Seq Scan on pgbench_accounts (cost=0.00..2640.00 rows=100000 width=0)
(2 rows)
robert.haas=#
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v2-0002-Allow-extensions-to-mark-an-individual-IndexOptIn.patchapplication/octet-stream; name=v2-0002-Allow-extensions-to-mark-an-individual-IndexOptIn.patchDownload+86-1
v2-0001-Convert-enable_-GUCs-into-per-RelOptInfo-values-w.patchapplication/octet-stream; name=v2-0001-Convert-enable_-GUCs-into-per-RelOptInfo-values-w.patchDownload+288-149
On 8/27/24 11:45, Robert Haas wrote:
On Mon, Aug 26, 2024 at 3:28 PM Robert Haas <robertmhaas@gmail.com> wrote:
Well, I agree that this doesn't address everything you might want to
do, ... I will very happily propose more things to
address the other problems that I know about ...In that vein, here's a new patch set where I've added a second patch
that allows extensions to control choice of index. It's 3 lines of new
code, plus 7 lines of comments and whitespace. Feeling inspired, I
also included a contrib module, initial_vowels_are_evil, to
demonstrate how this can be used by an extension that wants to disable
certain indexes but not others. This is obviously quite silly and we
might (or might not) want a more serious example in contrib, but it
demonstrates how easy this can be with just a tiny bit of core
infrastructure:robert.haas=# load 'initial_vowels_are_evil';
LOAD
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------
Aggregate (cost=2854.29..2854.30 rows=1 width=8)
-> Index Only Scan using pgbench_accounts_pkey on pgbench_accounts
(cost=0.29..2604.29 rows=100000 width=0)
(2 rows)
robert.haas=# alter index pgbench_accounts_pkey rename to
evil_pgbench_accounts_pkey;
ALTER INDEX
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
------------------------------------------------------------------------------
Aggregate (cost=2890.00..2890.01 rows=1 width=8)
-> Seq Scan on pgbench_accounts (cost=0.00..2640.00 rows=100000 width=0)
(2 rows)
robert.haas=#
Nice!
On the one hand, excluding indexes by initial vowels is definitely
silly. On the other, I can see how it might be useful for an extension
to exclude indexes based on a regex match of the index name or something
similar, at least for testing.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Tue, Aug 27, 2024 at 2:44 AM chungui.wcg <wcg2008zl@126.com> wrote:
I really admire this idea.
Thanks.
here is my confusion: Isn't the core of this idea whether to turn the planner into a framework? Personally, I think that under PostgreSQL's heap table storage, the optimizer might be better off focusing on optimizing the generation of execution plans. It’s possible that in some specific scenarios, developers might want to intervene in the generation of execution plans by extensions. I'm not sure if these scenarios usually occur when the storage structure is also extended by developers. If so, could existing solutions like "planner_hook" potentially solve the problem?
You could use planner_hook if you wanted to replace the entire planner
with your own planner. However, that doesn't seem like something
practical, as the planner code is very large. The real use of the hook
is to allow running some extra code when the planner is invoked, as
demonstrated by the pg_stat_statements contrib module. To get some
meaningful control over the planner, you need something more
fine-grained. You need to be able to run code at specific points in
the planner, as we already allow with, for example,
get_relation_info_hook or set_rel_pathlist_hook.
Whether or not that constitutes "turning the planner into a framework"
is, I suppose, a question of opinion. Perhaps a more positive way to
phrase it would be "allowing for some code reuse". Right now, if you
mostly like the behavior of the planner but want a few things to be
different, you've got to duplicate a lot of code and then hack it up.
That's not very nice. I think it's better to set things up so that you
can keep most of the planner behavior but override it in a few
specific cases without a lot of difficulty.
Cases where the data is stored in some different way are really a
separate issue from what I'm talking about here. In that case, you
don't want to override the planner behavior for all tables everywhere,
so planner_hook still isn't a good solution. You only want to change
the behavior for the specific table AM that implements the new
storage. You would probably want there to be an option where
cost_seqscan() calls a tableam-specific function instead of just doing
the same thing for every AM; and maybe something similar for indexes,
although that is less clear. The details aren't quite clear, which is
probably part of why we haven't done anything yet.
But this patch set is really more about enabling use cases where the
user wants an extension to take control of the plan more explicitly,
say to avoid some bad plan that they got before and that they don't
want to get again.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Aug 27, 2024 at 11:57 AM Joe Conway <mail@joeconway.com> wrote:
On the one hand, excluding indexes by initial vowels is definitely
silly. On the other, I can see how it might be useful for an extension
to exclude indexes based on a regex match of the index name or something
similar, at least for testing.
Right. I deliberately picked a contrib module that implemented a silly
policy, because what I wanted to demonstrate with it is that this
little bit of infrastructure provides enough mechanism to implement
whatever policy you want. And I think it demonstrates it quite well,
because the whole contrib module to implement this has just 6 lines of
executable code. If you wanted a policy that would be more
realistically useful, you'd need more code, but only however much is
needed to implement your policy. All you need do is replace this
strchr call with something else:
if (name != NULL && strchr("aeiouAEIOU", name[0]) != NULL)
index->disabled = true;
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
In that vein, here's a new patch set where I've added a second patch
that allows extensions to control choice of index.
I'm minus-several on this bit, because that is a solved problem and
we really don't need to introduce More Than One Way To Do It. The
intention has always been that get_relation_info_hook can editorialize
on the rel's indexlist by removing entries (or adding fake ones,
in the case of hypothetical-index extensions). For that matter,
if you really want to do it by modifying the IndexInfo rather than
deleting it from the list, that's already possible: just set
indexinfo->hypothetical = true.
regards, tom lane
On Tue, Aug 27, 2024 at 12:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
In that vein, here's a new patch set where I've added a second patch
that allows extensions to control choice of index.I'm minus-several on this bit, because that is a solved problem and
we really don't need to introduce More Than One Way To Do It. The
intention has always been that get_relation_info_hook can editorialize
on the rel's indexlist by removing entries (or adding fake ones,
in the case of hypothetical-index extensions). For that matter,
if you really want to do it by modifying the IndexInfo rather than
deleting it from the list, that's already possible: just set
indexinfo->hypothetical = true.
Well, now I'm confused. Just yesterday, in response to the 0001 patch
that allows extensions to exert control over the join strategy, you
complained that "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."
So today, I produced a patch demonstrating how we could address that
issue, and your response is "well, actually we don't need to do
anything about it because that problem is already solved." But if that
is true, then the fact that yesterday's patch did nothing about it was
a feature, not a bug.
Am I missing something here?
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
Well, now I'm confused. Just yesterday, in response to the 0001 patch
that allows extensions to exert control over the join strategy, you
complained that "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."
I was just using that to illustrate that making the enable_XXX GUCs
relation-local covers only a small part of the planner-control problem.
You had not, at that point, been very clear that you intended that
patch as only a small part of a solution.
I do think that index selection is pretty well under control already,
thanks to stuff that we put in ages ago at the urging of people who
wanted to write "index advisor" extensions. (The fact that that
area seems a bit moribund is disheartening, though. Is it a lack
of documentation?)
regards, tom lane
On Tue, Aug 27, 2024 at 2:24 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I was just using that to illustrate that making the enable_XXX GUCs
relation-local covers only a small part of the planner-control problem.
You had not, at that point, been very clear that you intended that
patch as only a small part of a solution.
Ah, OK, apologies for the lack of clarity. I actually think it's a
medium part of the solution. I believe the minimum viable product here
is probably something like:
- control over scan methods
- control over index selection
- control over join methods
- control over join order
It gets a lot better if we also have:
- control over aggregation methods
- something that I'm not quite sure about for appendrels
- control over whether parallelism is used and the degree of parallelism
If control over index selection is already adequate, then the proposed
patch is one way to get about 1/3 of the way to the MVP, which isn't
nothing. Maybe I'm underestimating the amount of stuff that people are
going to want here, but if you look at pg_hint_plan, it isn't doing a
whole lot more than this.
I do think that index selection is pretty well under control already,
thanks to stuff that we put in ages ago at the urging of people who
wanted to write "index advisor" extensions. (The fact that that
area seems a bit moribund is disheartening, though. Is it a lack
of documentation?)
So a couple of things about this.
First, EDB maintains closed-source index advisor code that uses this
machinery. In fact, if I'm not mistaken, we now have two extensions
that use it. So it's not dead from that point of view, but of course
anything closed-source can't be promoted through community channels.
There's open-source code around too; to my knowledge,
https://github.com/HypoPG/hypopg is the leading open-source
implementation, but my knowledge may very well be incomplete.
Second, I do think that the lack of documentation poses somewhat of a
challenge, and our exchange about whether an IndexOptInfo needs a
disabled flag is perhaps an example of that. To be fair, now that I
look at it, the comment where get_relation_info_hook does say that you
can remove indexes from the index list, so maybe I should have
realized that the problem can be solved that way, but on the other
hand, the comment for set_rel_pathlist_hook claims you can delete
paths from the pathlist, which AFAICS is completely non-viable, so one
can't necessarily rely too much on the comments in this area to learn
what actually does and does not work. Having some in-core examples
showing how to use this stuff correctly and demonstrating its full
power would also be really helpful. Right now, I often find myself
looking at out-of-core code which is sometimes poorly written and
frequently resorts to nasty hacks. It can be hard to determine whether
those nasty hacks are there because they're the only way to implement
some bit of functionality or because the author missed an opportunity
to do better.
Third, I think there's simply a lack of critical mass in terms of our
planner hooks. While the ability to add hypothetical indexes has some
use, the ability to remove indexes from consideration is probably
significantly more useful. But not if it's the only technique for
fixing a bad plan that you have available. Nobody gets excited about a
toolbox that contains just one tool. That's why I'm keen to expand
what can be done cleanly via hooks, and I think if we do that and also
provide either some very good documentation or some well-written
example implementations, we'll get more traction here.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, Aug 26, 2024 at 1:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
For example, I don't see
how this gets us any closer to letting an extension fix a poor choice
of join order.
Thinking more about this particular sub-problem, let's say we're
joining four tables A, B, C, and D. An extension wants to compel join
order A-B-C-D. Let's suppose, however, that it wants to do this in a
way where planning won't fail if that's impossible, so it wants to use
disabled_nodes rather than skipping path generation entirely.
When we're planning the baserels, we don't need to do anything
special. When we plan 2-way joins, we need to mark all paths disabled
except those originating from the A-B join. When we plan 3-way joins,
we need to mark all paths disabled except those originating from an
(A-B)-C join. When we plan the final 4-way join, we don't really need
to do anything extra: the only way to end up with a non-disabled path
at the top level is to pick a path from the (A-B)-C join and a path
from D.
There's a bit of nuance here, though. Suppose that when planning the
A-B join, the planner generates HashJoin(SeqScan(B),Hash(A)). Does
that path need to be disabled? If you think that join order A-B-C-D
means that table A should be the driving table, then the answer is
yes, because that path will lead to a join order beginning with B-A,
not one beginning with A-B. But you might also have a mental model
where it doesn't matter which side of the table is on which side of
the join, and as long as you start by joining A and B in some way,
that's good enough to qualify as an A-B join order. I believe actual
implementations vary in which approach they take.
I think that the beginning of add_paths_to_joinrel() looks like a
useful spot to get control. You could, for example, have a hook there
which returns a bitmask indicating which of merge-join, nested-loop,
and hash join will be allowable for this call; that hook would then
allow for control over the join method and the join order, and the
join order control is strong enough that you can implement either of
the two interpretations above. This idea theorizes that 0001 was wrong
to make the path mask a per-RelOptInfo value, because there could be
many calls to add_paths_to_joinrel() for a single RelOptInfo and, in
this idea, every one of those can enforce a different mask.
Potentially, such a hook could return additional information, either
by using more bits of the bitmask or by returning other information
via some other data type. For instance, I still believe that
distinguishing between parameterized-nestloops and
unparameterized-nestloops would be real darn useful, so we could have
separate bits for each; or you could have a bit to control whether
foreign-join paths get disabled (or are considered at all), or you
could have separate bits for merge joins that involve 0, 1, or 2
sorts. Whether we need or want any or all of that is certainly
debatable, but the point is that if you did want some of that, or
something else, it doesn't look difficult to feed that information
through to the places where you would need it to be available.
Thoughts?
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On Tue, Aug 27, 2024 at 03:11:15PM -0400, Robert Haas wrote:
Third, I think there's simply a lack of critical mass in terms of our
planner hooks. While the ability to add hypothetical indexes has some
use, the ability to remove indexes from consideration is probably
significantly more useful.
JFTR, hypopg can also mask away/hide indexes since version 1.4.0:
https://github.com/HypoPG/hypopg/commit/351f14a79daae8ab57339d2367d7f2fc639041f7
I haven't looked closely at the implementation though, and maybe you
meant something else in the above entirely.
Michael
Robert Haas <robertmhaas@gmail.com> writes:
I believe the minimum viable product here
is probably something like:
- control over scan methods
- control over index selection
- control over join methods
- control over join order
Seems reasonable. It might be possible to say that our answer
to "control over join order" is to provide a hook that can modify
the "joinlist" before it's passed to make_one_rel. If you want
to force a particular join order you can rearrange that
list-of-lists-of-range-table-indexes to do so. The thing this
would not give you is control over which rel is picked as outer
in any given join step. Not sure how critical that bit is.
regards, tom lane
On Tue, Aug 27, 2024 at 4:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Seems reasonable. It might be possible to say that our answer
to "control over join order" is to provide a hook that can modify
the "joinlist" before it's passed to make_one_rel. If you want
to force a particular join order you can rearrange that
list-of-lists-of-range-table-indexes to do so. The thing this
would not give you is control over which rel is picked as outer
in any given join step. Not sure how critical that bit is.
This has a big advantage over what I proposed yesterday in that it's
basically declarative. With one call to the hook, you get all the
information about the join order that you could ever want. That's
really nice. However, I don't really think it quite works, partly
because of what you mention here about not being able to control which
rel ends up on which side of the join, which I do think is important,
and also because if the join order isn't possible, planning will fail,
rather than falling back to some other plan shape. If you have an idea
how we could address those things within this same general framework,
I'd be keen to hear it.
It has occurred to me more than once that it might be really useful if
we could attempt to plan under a set of constraints and then, if we
don't end up finding a plan, retry without the constraints. But I
don't quite see how to make it work. When I tried to do that as a
solution to the disable_cost problem, it ended up meaning that once
you couldn't satisfy every constraint perfectly, you gave up on even
trying. I wasn't immediately certain that such behavior was
unacceptable, but I didn't have to look any further than our own
regression test suites to see that it was going to cause a lot of
unhappiness. In this case, if we could attempt join planning with the
user-prescribed join order and then try it again if we fail to find a
path, that would be really cool. Or if we could do all of planning
without generating disabled paths *at all* and then go back and
restart if it becomes clear that's not working out, that would be
slick. But, unless you have a clever idea, that all seems like
advanced magic that should wait until we have basic things working.
Right now, I think we should focus on getting something in place where
we still try all the paths but an extension can arrange for some of
them to be disabled. Then all the right things will happen naturally;
we'll only be leaving some CPU cycles on the table. Which isn't
amazing, but I don't think it's a critical defect either, and we can
try to improve things later if we want to.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi Robert,
On Mon, Aug 26, 2024 at 6:33 PM Robert Haas <robertmhaas@gmail.com> wrote:
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.
[..]
[..] But all that
said, as much as anything, I want to get some feedback on what
approaches and trade-offs people think might be acceptable here,
because there's not much point in me spending a bunch of time writing
code that everyone (or a critical mass of people) are going to hate.
As a somewhat tiny culprit of the self-flaming done by Robert (due to
nagging him about this in the past on various occasions), I'm of
course obligated to +1 to any work related to giving end-users/DBA the
ability to cage the plans generated by the optimizer.
When dealing with issues like those, I have a feeling we have 2
classes of most frequent issues being reported (that's my subjective
experience):
a. cardinality misestimate leading usually to nest loop plans (e.g.
JOIN estimates thread [1]/messages/by-id/c8c0ff31-3a8a-7562-bbd3-78b2ec65f16c@enterprisedb.com could also somehow help and it also has nice
reproducers)
b. issues after upgrades
So the "selectivity estimation hook(s)" mentioned by Andrei seems to
be a must, but also the ability not to just guess & tweak (shape) the
plan, but a way to export all SQL plans before upgrade with capability
to import and override(lock) specific SQL query to specific plan from
before upgrade.
I'm not into the internals of optimizer at all, but here are other
random thoughts/questions:
- I do think that "hints" words have bad connotations and should not
be used. It might be because of embedding them in SQL query text of
the application itself. On one front they are localized to the SQL
(good), but the PG operator has no realistic way of altering that once
it's embedded in binary (bad), as the application team is usually
separate if not from an external company (very bad situation, but
happens almost always).
- Would stacking of such extensions, each overriding the same planner
hooks, be allowed or not in the long run ? Technically there's nothing
preventing it and I think I could imagine someone attempting to run
multiple planner hook hotfixes for multiple issues, all at once?
- Shouldn't EXPLAIN contain additional information that for that
specific plan the optimizer hooks changed at least 1 thing ? (e.g.
"Plan was tainted" or something like that). Maybe extension could mark
it politely that it did by setting a certain flag, or maybe there
should be routines exposed by the core to do that ?
- the "b" (upgrade) seems like a much more heavy duty issue, as that
would require transfer of version-independent and textualized dump of
SQL plan that could be back-filled into a newer version of optimizer.
Is such a big thing realistic at all and it's better to just
concentrate on the hooks approach?
-J.
[1]: /messages/by-id/c8c0ff31-3a8a-7562-bbd3-78b2ec65f16c@enterprisedb.com