Re: PGdoc: add missing ID attribute to create_subscription.sgml

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, "Takamichi Osumi (Fujitsu)" <osumi(dot)takamichi(at)fujitsu(dot)com>
Subject: Re: PGdoc: add missing ID attribute to create_subscription.sgml
Date: 2023-03-28 01:08:18
Message-ID: CAHut+PtDBQHJW2toVF3VZBLyfWOCNi5XW3yBqVUT0XnysCvVQw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Kuroda-san. Here are my review comments for both patches v3-0001 and v3-0002.

////////
v3-0001
////////

This patch looks good, but I think there are a couple of other places
where you could add links:

~~~

1.1 doc/src/sgml/logical-replication.sgml (31.5 Conflicts)

"When the streaming mode is parallel, the finish LSN ..."

Maybe you can add a "streaming" link there.

~~~

1.2. doc/src/sgml/logical-replication.sgml (31.5 31.8. Monitoring)

"Moreover, if the streaming transaction is applied in parallel, there
may be additional parallel apply workers."

Maybe you can add a "streaming" link there.

////////
v3-0002
////////

There is one bug, and I think there are a couple of other places where
you could add links:

~~~

2.1 doc/src/sgml/logical-replication.sgml (31.4. Column Lists blurb)

For partitioned tables, the publication parameter
publish_via_partition_root determines which column list is used.

~

Maybe you can add a "publish_via_partition_root" link there.

~~~

2.2 doc/src/sgml/logical-replication.sgml (31.6. Restrictions)

Publications can also specify that changes are to be replicated using
the identity and schema of the partitioned root table instead of that
of the individual leaf partitions in which the changes actually
originate (see CREATE PUBLICATION).

~

Maybe that text can be changed now to say something like "(see
publish_via_partition_root parameter of CREATE PUBLICATION)” -- so
only the parameter part has the link, not the CREATE PUBLICATION part.

~~~

2.3 doc/src/sgml/logical-replication.sgml (31.9. Security)

+ subscription <link
linkend="sql-createpublication-for-all-tables"><literal>FOR ALL
TABLES</literal></link>
+ or <link linkend="sql-createpublication-for-tables-in-schema"><literal>FOR
TABLES IN SCHEMA</literal></link><literal>FOR TABLES IN
SCHEMA</literal>
+ only when superusers trust every user permitted to create a non-temp table
+ on the publisher or the subscriber.

There is a cut/paste typo here -- it renders badly with "FOR TABLES IN
SCHEMA" appearing 2x.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2023-03-28 01:35:38 Re: Moving forward with TDE
Previous Message Peter Geoghegan 2023-03-28 01:07:09 Re: Add pg_walinspect function with block info columns