Re: DOCS: add helpful partitioning links - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: DOCS: add helpful partitioning links
Date
Msg-id CAExHW5uwD0hE+ezMej7r9B6MQ2R4Gm0T2J6YFSDs8SgtPzwvpA@mail.gmail.com
Whole thread Raw
In response to Re: DOCS: add helpful partitioning links  (Robert Treat <rob@xzilla.net>)
Responses Re: DOCS: add helpful partitioning links
List pgsql-hackers
Hi Robert,


On Mon, Mar 18, 2024 at 10:52 PM Robert Treat <rob@xzilla.net> wrote:
On Thu, Mar 14, 2024 at 12:15 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> Hi Robert,
>
> On Thu, Mar 7, 2024 at 10:49 PM Robert Treat <rob@xzilla.net> wrote:
>>
>> This patch adds a link to the "attach partition" command section
>> (similar to the detach partition link above it) as well as a link to
>> "create table like" as both commands contain additional information
>> that users should review beyond what is laid out in this section.
>> There's also a couple of wordsmiths in nearby areas to improve
>> readability.
>
>
> Thanks.
>
> The patch gives error when building html
>
> ddl.sgml:4300: element link: validity error : No declaration for attribute linked of element link
>      <link linked="sql-createtable-parms-like"><literal>CREATE TABLE ... LIKE</l
>                                               ^
> ddl.sgml:4300: element link: validity error : Element link does not carry attribute linkend
> nked="sql-createtable-parms-like"><literal>CREATE TABLE ... LIKE</literal></link
>                                                                                ^
> make[1]: *** [Makefile:72: postgres-full.xml] Error 4
> make[1]: *** Deleting file 'postgres-full.xml'
> make[1]: Leaving directory '/home/ashutosh/work/units/pg_review/coderoot/pg/doc/src/sgml'
> make: *** [Makefile:8: html] Error 2
>
> I have fixed in the attached.
>

Doh! Thanks!

>
> -     As an alternative, it is sometimes more convenient to create the
> -     new table outside the partition structure, and attach it as a
> +     As an alternative, it is sometimes more convenient to create a
> +     new table outside of the partition structure, and attach it as a
>
> it uses article "the" for "new table" since it's referring to the partition mentioned in the earlier example. I don't think using "a" is correct.
>

I think this section has a general problem of mingling the terms table
and partition in a way they can be confusing.

In this case, you have to infer that the term "the new table" is
referring not to the only table mentioned in the previous paragraph
(the partitioned table), but actually to the partition mentioned in
the previous paragraph. For long term postgres folks the idea that
partitions are tables isn't a big deal, but that isn't how folks
coming from other databases see things. So I lean towards "a new
table" because we are specifically talking about an alternative to the
above paragraph... ie. don't make a new partition, make a new table.
And tbh I think that wording (create a new table and attach it as a
partition) is still better than the wording in your patch, because the
"new partition" you are creating isn't a partition until it is
attached; it is just a new table.

> "outside" seems better than "outside of". See https://english.stackexchange.com/questions/9700/outside-or-outside-of. But I think the meaning of the sentence will be more clear if we rephrase it as in the attached patch.
>

This didn't really clarify anything for me, as the discussion in that
link seems to be around the usage of the term wrt physical location,
and it is much less clear about the context of a logical construct.
Granted, your patch removes that, though I think now I'd lean toward
using the phrase "separate from".

> -     convenient, as not only will the existing partitions become indexed, but
> -     also any partitions that are created in the future will.  One limitation is
> +     convenient as not only will the existing partitions become indexed, but
> +     any partitions created in the future will as well.  One limitation is
>
> I am finding the current construct hard to read. The comma is misplaced as you have pointed out. The pair of commas break the "not only" ... "but also" construct. I have tried to simplify the sentence in the attached. Please review.
>
> -     the partitioned table; such an index is marked invalid, and the partitions
> -     do not get the index applied automatically.  The indexes on partitions can
> -     be created individually using <literal>CONCURRENTLY</literal>, and then
> +     the partitioned table; such an index is marked invalid and the partitions
> +     do not get the index applied automatically.  The partition indexes can
>
> "indexes on partition" is clearer than "partition index". Fixed in the attached patch.
>
> Please review.

The language around all this is certainly tricky (like, what is a
partitioned index vs parent index?), and one thing I'd certainly try
to avoid is using any words like "inherited" which is also overloaded
in this context. In any case, I took in all the above and had a stab
at a v3


The patch doesn't apply cleanly
$ git apply /tmp/improve-partition-links_v3.patch
error: patch failed: doc/src/sgml/ddl.sgml:4266
error: doc/src/sgml/ddl.sgml: patch does not apply

$ patch -p1 < /tmp/improve-partition-links_v3.patch
patching file doc/src/sgml/ddl.sgml
Hunk #1 FAILED at 4266.
Hunk #2 succeeded at 4333 (offset 12 lines).
Hunk #3 FAILED at 4332.
2 out of 3 hunks FAILED -- saving rejects to file doc/src/sgml/ddl.sgml.rej

+     As an alternative to creating new partitions, it is sometimes more

edit: creating a new partition .. rest of the sentence is in singular.

+     convenient to create a new table seperate from the partition structure
+     and attach it as a partition later. This allows new data to be loaded,
+     checked, and transformed prior to it appearing in the partitioned table.

Rest of it looks good to me.

Please add it to the next commitfest. Most likely the patch will be considered for PG 17 itself, but we won't forget it if it's in CF.

--
Best Wishes,
Ashutosh Bapat

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: Reducing connection overhead in pg_upgrade compat check phase
Next
From: Bertrand Drouvot
Date:
Subject: Re: Autogenerate some wait events code and documentation