Thanks for looking at this.
On Tue, 11 Jun 2019 at 01:44, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> part_doc_pg10_v5.patch :
> + query planning and execution. The query planner is generally able to
> + handle partition hierarchies up a few hundred partition. Planning times
>
> "up TO a few hundred partition*S*" ?
Oops. My backspace key must have removed too many chars when I removed
"quite well" out of the PG10 version.
> part_doc_master_v5.patch:
> + Choosing the target number of partitions into which the table should be
> + divided by is also a critical decision to make.
>
> "into which ... should be divided by" seems like a copy-editing
> mistake.
Yes it is. It only existed in the master version. I'm not sure how it
snuck by in there.
> Did you mean to remove either the "into which" or the "by"?
I meant to remove "by", per advice from Justin.
> I think "the target number of partitions THAT the table should be
> divided into" is simple and sensible; I'm not sure I trust the version
> with "into which" instead of "that", and the role of "by" is not clear
> to me ("divide by" implies a divisor, but here we're talking about the
> resulting chunks and not the divisor).
This is tricky. Justin liked it that way and since it took me a few
rounds to get it the way he wanted, I'm quite tempted by the
status-quo.
> In this phrase (all versions):
> + That's because each partition requires its own metadata that must be
> + loaded into the local memory of each session that touches it.
>
> I would replace "requires its own metadata that must be loaded" with
> "requires its metadata to be loaded".
That seems like a good improvement. Changed to that.
v6 versions are attached.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services