Re: Add mention of execution time memory for enable_partitionwise_* GUCs - Mailing list pgsql-hackers

On Tue, Jul 30, 2024 at 5:38 PM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Tue, 30 Jul 2024 at 21:12, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> > Thanks. This looks better. Nitpick
> >
> > +        child partitions.  With this setting enabled, the number of executor
> > +        nodes whose memory usage is restricted by <varname>work_mem</varname>
> >
> > This sentence appears to say that the memory usage of "all" nodes is
> > restricted by work_mem. I think what you want to convey is - nodes,
> > whose memory usage is subjected to <varname>work_mem</varname>
> > setting, ....
>
> I'm open to improving the sentence but I don't quite follow why
> "subjected to" is better than "restricted by". It seems to remove
> meaning without saving any words. With "restricted by" we can deduce
> that the memory cannot go over work_mem, whereas "subjected to" only
> gives us some indication that the memory usage somehow relates to the
> work_mem setting and doesn't not mean that the memory used could be >=
> work_mem.
>
> > Or break it into two sentences
> >
> > With this setting enabled, the number of executor nodes appearing in
> > the final plan can increase linearly proportional to the number of
> > partitions being scanned. Each of those nodes may use upto
> > <varname>work_mem</varname> memory. This can ...
>
> ... but that's incorrect. This means that all additional nodes that
> appear in the plan as a result of enabling the setting can use up to
> work_mem.  That's not the case as some of the new nodes might be
> Nested Loops or Merge Joins and they're not going to use up to
> work_mem.

Any wording, which indicates that "some" of those nodes "may" use upto
"work_mem" memory "each", is fine with me. If you think that your
current wording conveys that meaning, I am ok.

>
> I much prefer "nodes" to "operations".  If someone started asking me
> about "query operations", I'd have to confirm what they meant. An I/O
> is an operation that can occur during the execution of a query. Is
> that a "query operation"? IMO, the wording you're proposing is less
> concise. I'm not a fan of the terminology when talking about plan
> nodes. I'd rather see the work_mem docs modified than copy what it
> says.

We need to use consistent terms at both places. Somebody reading the
new text and then referring to work_mem description will wonder
whether "query operations" are the same thing as "executor nodes" or
not. But I get your point that query operation doesn't necessarily
mean operations performed by each executor node, especially when those
operations are not specified directly in the query. We can commit your
version and see if users find it confusing. May be they already know
that operations and nodes are interchangeable in this context.

I noticed a previous discussion about work_mem documentation [1]. But
that didn't change the first sentence in the description.

[1] https://www.postgresql.org/message-id/66590882-F48C-4A25-83E3-73792CF8C51F%40amazon.com


--
Best Wishes,
Ashutosh Bapat