Thread: Should work_mem be stable for a prepared statement?

Should work_mem be stable for a prepared statement?

From
Jeff Davis
Date:
As a part of this discussion:

https://www.postgresql.org/message-id/CAJVSvF6s1LgXF6KB2Cz68sHzk%2Bv%2BO_vmwEkaon%3DH8O9VcOr-tQ%40mail.gmail.com

James pointed out something interesting, which is that a prepared
statement enforces the work_mem limit at execution time, which might be
different from the work_mem at the time the statement was prepared.

For instance:

   SET work_mem='1GB';
   PREPARE foo AS ...; -- plans using 1GB limit
   SET work_mem='1MB';
   EXECUTE foo; -- enforces 1MB limit

My first reaction is that it's not right because the costing for the
plan is completely bogus with a different work_mem. It would make more
sense to me if we either (a) enforced work_mem as it was at the time of
planning; or (b) replanned if executed with a different work_mem
(similar to how we replan sometimes with different parameters).

But I'm not sure whether someone might be relying on the existing
behavior?

If we were to implement (a) or (b), we couldn't use the work_mem global
directly, we'd need to save it in the plan, and enforce using the
plan's saved value. But that might be a good change anyway. In theory
we might need to do something similar for hash_mem_multiplier, too.

Regards,
    Jeff Davis




Re: Should work_mem be stable for a prepared statement?

From
Greg Sabino Mullane
Date:
On Thu, Feb 27, 2025 at 1:42 PM Jeff Davis <pgsql@j-davis.com> wrote:
It would make more sense to me if we either (a) enforced work_mem as it was at the time of planning; or (b) replanned if executed with a different work_mem (similar to how we replan sometimes with different parameters).

Definitely (b). 

But I'm not sure whether someone might be relying on the existing behavior?

I cannot fathom a reason why.

Cheers,
Greg

--
Enterprise Postgres Software Products & Tech Support

Re: Should work_mem be stable for a prepared statement?

From
David Rowley
Date:
On Fri, 28 Feb 2025 at 07:42, Jeff Davis <pgsql@j-davis.com> wrote:
> https://www.postgresql.org/message-id/CAJVSvF6s1LgXF6KB2Cz68sHzk%2Bv%2BO_vmwEkaon%3DH8O9VcOr-tQ%40mail.gmail.com
>
> James pointed out something interesting, which is that a prepared
> statement enforces the work_mem limit at execution time, which might be
> different from the work_mem at the time the statement was prepared.

There's a similar but not quite the same situation with the enable_*
GUCs. The executor isn't going to pick up a new value for these like
it will for work_mem, but I think portions of the same argument can be
made, i.e. Someone might not like that turning off enable_seqscan
after doing PREPARE and EXECUTE once does not invalidate their plan.

> My first reaction is that it's not right because the costing for the
> plan is completely bogus with a different work_mem. It would make more
> sense to me if we either (a) enforced work_mem as it was at the time of
> planning; or (b) replanned if executed with a different work_mem
> (similar to how we replan sometimes with different parameters).

If we were to fix this then a) effectively already happens for the
enable_* GUCs, so b) would be the only logical way to fix.

> But I'm not sure whether someone might be relying on the existing
> behavior?

It looks like there was a bit of discussion on this topic about 18
years ago in [1], but it didn't seem to end with a very conclusive
outcome. I did learn that we once didn't have a method to invalidate
cached plans, so perhaps the current behaviour is a remnant of the
previous lack of infrastructure.

David

[1] https://www.postgresql.org/message-id/15168.1174410673%40sss.pgh.pa.us



Re: Should work_mem be stable for a prepared statement?

From
Tom Lane
Date:
David Rowley <dgrowleyml@gmail.com> writes:
> On Fri, 28 Feb 2025 at 07:42, Jeff Davis <pgsql@j-davis.com> wrote:
>> My first reaction is that it's not right because the costing for the
>> plan is completely bogus with a different work_mem. It would make more
>> sense to me if we either (a) enforced work_mem as it was at the time of
>> planning; or (b) replanned if executed with a different work_mem
>> (similar to how we replan sometimes with different parameters).

> If we were to fix this then a) effectively already happens for the
> enable_* GUCs, so b) would be the only logical way to fix.

Given that nobody's complained about this for twenty-plus years,
I can't get excited about adding complexity to do either thing.

            regards, tom lane



Re: Should work_mem be stable for a prepared statement?

From
Jeff Davis
Date:
On Thu, 2025-02-27 at 17:04 -0500, Tom Lane wrote:
> Given that nobody's complained about this for twenty-plus years,
> I can't get excited about adding complexity to do either thing.

I had in mind some refactoring in this area, which ideally would not
add complexity. It might provide some nice benefits, but would
introduce this behavior change, which makes it slightly more than a
refactoring.

It sounds like the behavior change would be desirable or at least
neutral. I will have to try it out and see if the refactoring is a net
improvement or turns into a mess.

Regards,
    Jeff Davis




Re: Should work_mem be stable for a prepared statement?

From
Sami Imseih
Date:
> It sounds like the behavior change would be desirable or at least
> neutral. I will have to try it out and see if the refactoring is a net
> improvement or turns into a mess.

I think this is a good operational improvement, particularly if
someone wants to change work_mem in a pinch, and the only
option now they have it to somehow get the application to
re-prepare; deallocating all prepared statements or reconnecting.
This is even worse with extended query protocol prepared statements
in which there is no visibility in pg_prepared_statements. So one may
be forced to use DEALLOCATE ALL.

However, I think any GUC that can influence the planner
should be considered for consistency in behavior.
It was mentioned above with the enable_* GUCs, but another
one I can think of is the max_parallel_workers_per_gather which
should then force a re-plan if changed. I have seen users need to turn
that off in a hurry when it impacts their oltp workload.


--
Sami Imseih
Amazon Web Services (AWS)



Re: Should work_mem be stable for a prepared statement?

From
Tom Lane
Date:
Sami Imseih <samimseih@gmail.com> writes:
> However, I think any GUC that can influence the planner
> should be considered for consistency in behavior.
> It was mentioned above with the enable_* GUCs, but another
> one I can think of is the max_parallel_workers_per_gather which
> should then force a re-plan if changed. I have seen users need to turn
> that off in a hurry when it impacts their oltp workload.

The argument for treating work_mem specially is that it has effects at
both plan time and run time, so that the planner's cost assumptions
are invalidated if the executor uses a different value than the
planner did.  I don't believe that argument applies to anything else;
certainly it doesn't apply to the enable_* flags.  I'm also not
convinced that the argument requires us to solve the problem by
re-planning.  It would work just as well to stamp each PlannedStmt
with the value that the planner used and refer to that in the
executor instead of looking directly at the GUC.

This is all kind of moot though, now that Jeff has revealed that
what he's really interested in is some sort of refactoring.
Maybe that refactoring is one that would conveniently apply to
other GUCs, or maybe it isn't.  I'm content to await details
before arguing about what we "should" do.

            regards, tom lane



Re: Should work_mem be stable for a prepared statement?

From
Sami Imseih
Date:
> The argument for treating work_mem specially is that it has effects at
> both plan time and run time, so that the planner's cost assumptions
> are invalidated if the executor uses a different value than the
> planner did.

I see that now. Thanks!

> Maybe that refactoring is one that would conveniently apply to
> other GUCs, or maybe it isn't.  I'm content to await details
> before arguing about what we "should" do.

Agree.

--
Sami



Re: Should work_mem be stable for a prepared statement?

From
Sami Imseih
Date:
> I'm also not convinced that the argument requires us to solve
> the problem by re-planning.  It would work just as well to stamp
> each PlannedStmt with the value that the planner used and
> refer to that in the executor instead of looking directly at the GUC.

hmm, if work_mem influences the plan, can we really avoid re-planning?
Here is an example where re-plannning is required to yield a plan
that is based on the current work_mem. right?

postgres=# show work_mem ;
 work_mem
----------
 4MB
(1 row)

postgres=# prepare sortprep as select * from pg_class order by oid ;
PREPARE
postgres=# explain execute sortprep;
                            QUERY PLAN
-------------------------------------------------------------------
 Sort  (cost=36.20..37.23 rows=415 width=273)
   Sort Key: oid
   ->  Seq Scan on pg_class  (cost=0.00..18.15 rows=415 width=273)
(3 rows)

postgres=# set work_mem = "64kB";
SET
postgres=# explain execute sortprep;
                            QUERY PLAN
-------------------------------------------------------------------
 Sort  (cost=36.20..37.23 rows=415 width=273)
   Sort Key: oid
   ->  Seq Scan on pg_class  (cost=0.00..18.15 rows=415 width=273)
(3 rows)

postgres=# explain  select * from pg_class order by oid ;
                                       QUERY PLAN
----------------------------------------------------------------------------------------
 Index Scan using pg_class_oid_index on pg_class  (cost=0.27..60.85
rows=415 width=273)
(1 row)

--

Sami



Re: Should work_mem be stable for a prepared statement?

From
James Hunter
Date:
On Fri, Feb 28, 2025 at 2:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Sami Imseih <samimseih@gmail.com> writes:
> > However, I think any GUC that can influence the planner
> > should be considered for consistency in behavior.
> > It was mentioned above with the enable_* GUCs, but another
> > one I can think of is the max_parallel_workers_per_gather which
> > should then force a re-plan if changed. I have seen users need to turn
> > that off in a hurry when it impacts their oltp workload.
>
> The argument for treating work_mem specially is that it has effects at
> both plan time and run time, so that the planner's cost assumptions
> are invalidated if the executor uses a different value than the
> planner did.  I don't believe that argument applies to anything else;
> certainly it doesn't apply to the enable_* flags.

I take the view:
 * for the plan time effects, the point of a prepared statement is to
plan exactly once, at the time the statement is prepared. So I don't
think it makes sense to replan when a GUC changes. (So, no to option
(b).)
* for the run time effects, the current run time state of the world
should control. (So, no to option (a).)

But I would leave (a) open as an option for extensions.

>  I'm also not
> convinced that the argument requires us to solve the problem by
> re-planning.  It would work just as well to stamp each PlannedStmt
> with the value that the planner used and refer to that in the
> executor instead of looking directly at the GUC.

I propose something similar, in [1], except that instead of stamping
the PlannedStmt with a single (pair of) GUC value, I stamp it with a
separate value for every Plan or SubPlan that needs it. And then a
hook / extension can impose Jeff's option (a), which is what you
describe here.

> This is all kind of moot though, now that Jeff has revealed that
> what he's really interested in is some sort of refactoring.
> Maybe that refactoring is one that would conveniently apply to
> other GUCs, or maybe it isn't.  I'm content to await details
> before arguing about what we "should" do.

I think refactoring is good (as you'd expect, given my proposal!), but
-- expanding on what you wrote above -- I think it's also important to
consider "plan time effects" and "run time effects" separately. The
same GUC / field / whatever might sometimes be used at both plan and
run time, but conceptually these are different.

A plan-time setting is intended to bias the planner so that it chooses
certain classes of plans over others. A plan-time setting is "wrong"
only so far as it causes the planner to choose a sub-optimal plan.

A run-time setting reflects the current state of the PostgreSQL
instance, at the time the query is run.

PQ is a good example. At plan time, we choose PQ, and place a Gather
node on the Plan, based on an assumption
(max_parallel_workers_per_gather) about how much parallelism we'll
actually get at run-time. And then, at run-time, we assign actual
workers based on max_parallel_workers and other queries running at the
same time.

It may turn out that we can't actually get *any* actual workers, at
runtime; so then we have added a Gather node for nothing. This has a
non-zero cost. If we had known, at plan time, that we wouldn't get any
parallel workers, then we should have chosen a serial plan. But, so
what? Planning is always an estimate.

Yeah, "Gather" overhead is lower than the cost of choosing a
sub-optimal join method, but I think the same principle applies:
* at plan time, we make the best estimate and prediction we can, based
on info available at that time;
* at run time, we make the best decision we can, based on the actual
state of the database instance.

And in cases where we think, at runtime, that the planner's
assumptions were so wrong that they'll lead us to execute a
sub-optimal plan, then maybe we can re-plan. But I explicitly wouldn't
re-plan a prepared statement, since the customer's expectation is that
it has already been prepared.

James Hunter

[1]
https://www.postgresql.org/message-id/flat/CAJVSvF5n3_uEGW5GZSRehDuTfz7XVDohbn7tVJ%2B2ZnweQEVFrQ%40mail.gmail.com#abc6e69a396bb9f6505bf33260670a1f