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

Attachment: signature.asc
Description: PGP signature

Reply via email to