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
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 |