Re: doc review for v14 - Mailing list pgsql-hackers

On Mon, Dec 21, 2020 at 10:11:53PM -0600, Justin Pryzby wrote:
> As I did last 2 years, I reviewed docs for v14...

Thanks for gathering all that!

> This year I've started early, since it takes more than a little effort and it's
> not much fun to argue the change in each individual hunk.

0001-pgindent-typos.not-a-patch touches pg_bsd_indent.

>      /*
> -     * XmlTable returns table - set of composite values. The error context, is
> -     * used for producement more values, between two calls, there can be
> -     * created and used another libxml2 error context. It is libxml2 global
> -     * value, so it should be refreshed any time before any libxml2 usage,
> -     * that is finished by returning some value.
> +     * XmlTable returns a table-set of composite values. The error context is
> +     * used for providing more detail. Between two calls, other libxml2
> +     * error contexts might have been created and used ; since they're libxml2
> +     * global values, they should be refreshed each time before any libxml2 usage
> +     * that finishes by returning some value.
>       */

That's indeed incorrect, but I am not completely sure if what you have
here is correct either.  I'll try to study this code a bit more first,
though I have said that once in the past.  :p

> --- a/src/bin/pg_dump/pg_restore.c
> +++ b/src/bin/pg_dump/pg_restore.c
> @@ -305,7 +305,7 @@ main(int argc, char **argv)
>      /* Complain if neither -f nor -d was specified (except if dumping TOC) */
>      if (!opts->cparams.dbname && !opts->filename && !opts->tocSummary)
>      {
> -        pg_log_error("one of -d/--dbname and -f/--file must be specified");
> +        pg_log_error("one of -d/--dbname, -f/--file or -l/--list must be specified");
>          exit_nicely(1);
>      }

You have forgotten to update the TAP test pg_dump/t/001_basic.pl.
The message does not seem completely incorrect to me either.  Hmm.
Restraining more the set of options is something to consider, though
it could be annoying.  I have discarded this one for now.

>          Specifies the amount of memory that should be allocated at server
> -        startup time for use by parallel queries.  When this memory region is
> +        startup for use by parallel queries.  When this memory region is
>          insufficient or exhausted by concurrent queries, new parallel queries
>          try to allocate extra shared memory temporarily from the operating
>          system using the method configured with
>          <varname>dynamic_shared_memory_type</varname>, which may be slower due
>          to memory management overheads.  Memory that is allocated at startup
> -        time with <varname>min_dynamic_shared_memory</varname> is affected by
> +        with <varname>min_dynamic_shared_memory</varname> is affected by
>          the <varname>huge_pages</varname> setting on operating systems where
>          that is supported, and may be more likely to benefit from larger pages
>          on operating systems where that is managed automatically.

The current formulation is not that confusing, but I agree that this
is an improvement.  Thomas, you are behind this one.  What do you
think?

I have applied most of it on HEAD, except 0011 and the things noted
above.  Thanks again.
--
Michael