Re: doc review for v14

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Bruce Momjian <bruce(at)momjian(dot)us>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: doc review for v14
Date: 2020-12-24 08:12:02
Message-ID: X+RNUu+zotOjfL7K@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2020-12-24 08:39:08 Re: Consider Parallelism While Planning For REFRESH MATERIALIZED VIEW
Previous Message Kyotaro Horiguchi 2020-12-24 08:02:20 Re: In-placre persistance change of a relation