Обсуждение: Various typo fixes

Hi,

I've attached a patch with a few typo and grammatical fixes.

Regards

Thom

On Tue, Apr 11, 2023 at 03:36:02PM +0100, Thom Brown wrote:
> I've attached a patch with a few typo and grammatical fixes.

I think you actually sent the "git-diff" manpage :(

-- 
Justin

On Tue, 11 Apr 2023 at 15:39, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Tue, Apr 11, 2023 at 03:36:02PM +0100, Thom Brown wrote:
> > I've attached a patch with a few typo and grammatical fixes.
>
> I think you actually sent the "git-diff" manpage :(

Oh dear, well that's a first.  Thanks for pointing out.

Re-attached.

Thom

On Tue, Apr 11, 2023 at 03:43:12PM +0100, Thom Brown wrote:
> On Tue, 11 Apr 2023 at 15:39, Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Tue, Apr 11, 2023 at 03:36:02PM +0100, Thom Brown wrote:
> > > I've attached a patch with a few typo and grammatical fixes.
> >
> > I think you actually sent the "git-diff" manpage :(
> 
> Oh dear, well that's a first.  Thanks for pointing out.

Thanks.  I think these are all new in v16, right ?

I noticed some of these too - I'll send a patch pretty soon.

|+++ b/doc/src/sgml/logicaldecoding.sgml
|@@ -326,11 +326,11 @@ postgres=# select * from pg_logical_slot_get_changes('regression_slot', NULL, NU
|      connection is alive (for example a node restart would break it). Then, the
|      primary may delete system catalog rows that could be needed by the logical
|      decoding on the standby (as it does not know about the catalog_xmin on the
|-     standby). Existing logical slots on standby also get invalidated if wal_level
|-     on primary is reduced to less than 'logical'. This is done as soon as the
|-     standby detects such a change in the WAL stream. It means, that for walsenders
|-     that are lagging (if any), some WAL records up to the wal_level parameter change
|-     on the primary won't be decoded.
|+     standby). Existing logical slots on standby also get invalidated if
|+     <varname>wal_level</varname> on the primary is reduced to less than 'logical'.
|+     This is done as soon as the standby detects such a change in the WAL stream.
|+     It means that, for walsenders which are lagging (if any), some WAL records up
|+     to the wal_level parameter change on the primary won't be decoded.
|     </para>

I think "logical" should be a <literal> here.

> On 11 Apr 2023, at 16:53, Justin Pryzby <pryzby@telsasoft.com> wrote:

> I think "logical" should be a <literal> here.

Agree, it should in order to be consistent.

--
Daniel Gustafsson

On Tue, Apr 11, 2023 at 09:53:00AM -0500, Justin Pryzby wrote:
> On Tue, Apr 11, 2023 at 03:43:12PM +0100, Thom Brown wrote:
> > On Tue, 11 Apr 2023 at 15:39, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > >
> > > On Tue, Apr 11, 2023 at 03:36:02PM +0100, Thom Brown wrote:
> > > > I've attached a patch with a few typo and grammatical fixes.
> > >
> > > I think you actually sent the "git-diff" manpage :(
> > 
> > Oh dear, well that's a first.  Thanks for pointing out.
> 
> Thanks.  I think these are all new in v16, right ?
> 
> I noticed some of these too - I'll send a patch pretty soon.

The first attachment fixes for typos in user-facing docs new in v16,
combining Thom's changes with the ones that I'd found.  If that's
confusing, I'll resend my patches separately.

The other four numbered patches could use extra review.

-- 
Justin

On Tue, Apr 11, 2023 at 11:12:58PM +0200, Daniel Gustafsson wrote:
> > On 11 Apr 2023, at 16:53, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> > I think "logical" should be a <literal> here.
>
> Agree, it should in order to be consistent.

Indeed.

+     to the wal_level parameter change on the primary won't be decoded.

This wal_level should also have a markup.

        Number of uses of logical slots in this database that have been
-       canceled due to old snapshots or a too low <xref linkend="guc-wal-level"/>
+       canceled due to old snapshots or too low a <xref linkend="guc-wal-level"/>

This sounds a bit strange to me.  A too low wal_level would be a cause
for a cancel, hence shouldn't this be "canceled due to old snapshots
or due to a too low guc-wal-level?
--
Michael

On Wed, Apr 12, 2023 at 12:28:25PM +0900, Michael Paquier wrote:
> On Tue, Apr 11, 2023 at 11:12:58PM +0200, Daniel Gustafsson wrote:
> > > On 11 Apr 2023, at 16:53, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > 
> > > I think "logical" should be a <literal> here.
> > 
> > Agree, it should in order to be consistent.
> 
> Indeed.
> 
> +     to the wal_level parameter change on the primary won't be decoded.
> 
> This wal_level should also have a markup.
> 
>         Number of uses of logical slots in this database that have been
> -       canceled due to old snapshots or a too low <xref linkend="guc-wal-level"/>
> +       canceled due to old snapshots or too low a <xref linkend="guc-wal-level"/>
> 
> This sounds a bit strange to me.  A too low wal_level would be a cause
> for a cancel, hence shouldn't this be "canceled due to old snapshots
> or due to a too low guc-wal-level?

That's the same as the original language which Thom and I are requesting
to change, (but you added another "due to").

"a too low" is poor english.  It's good enough for a code comment, but
this is a user-facing doc.

It could be "an inadequate wal-level" or "a prohibitively low
wal-level", but Thom's language is better.  "too low a wal-level" means
the same thing as "too low of a wal-level" (which would also be fine).

-- 
Justin

On Tue, Apr 11, 2023 at 05:15:29PM -0500, Justin Pryzby wrote:
> The first attachment fixes for typos in user-facing docs new in v16,
> combining Thom's changes with the ones that I'd found.  If that's
> confusing, I'll resend my patches separately.
>
> The other four numbered patches could use extra review.

In v16-typos.diff..

-        <literal>buffered</literal>, the decoding will stream or serialize
+        <literal>buffered</literal>, decoding will stream or serialize

The could be referred as "the decoding context", as well?

-   not starting with a <literal>.</literal> and ending with
-   <literal>.conf</literal> will be included. Multiple files within an include
+   ending with <literal>.conf</literal> and not starting with a <literal>.</literal>
+   will be included. Multiple files within an include

In 0001..  Not sure that this is an improvement, switching the
starting and ending parts.

-   include records. These records only contain two fields:
+   include directives. Include directives only contain two fields:
[...]
-   included. The file or directory can be a relative of absolute path, and can
+   included. The file or directory can be a relative or absolute path, and can

Yep, indeed.

We've discussed quite a lot about the current wording that 0004 aims
to change, FWIW.

I have applied a first batch of fixes that relate to the areas
introduced by myself, plus a few extras.  The changes for
pg_log_standby_snapshot() mostly left out for now (except one simple
change in logicaldecoding.sgml).

Most of the changes in 0002 and 0003 seem rather OK at quick glance,
but perhaps their respective authors would like to weigh in.
--
Michael

On Tue, Apr 11, 2023 at 10:44:43PM -0500, Justin Pryzby wrote:
> It could be "an inadequate wal-level" or "a prohibitively low
> wal-level", but Thom's language is better.  "too low a wal-level" means
> the same thing as "too low of a wal-level" (which would also be fine).

I have been studying more this point, and you are right that this is
much better.  So applied with this wording, after adding more markups
where these were needed.
--
Michael