Re: doc review for v13
Justin Pryzby writes: > Thanks. Here's the remainder, with some new ones. LGTM. I tweaked one or two places a bit more, and pushed it. regards, tom lane
Re: doc review for v13
On Thu, Sep 10, 2020 at 03:58:31PM +0900, Michael Paquier wrote: > On Wed, Sep 09, 2020 at 09:37:42AM -0500, Justin Pryzby wrote: > > I've added a few more. > > I have done an extra round of review on this patch series, and applied > what looked obvious to me (basically the points already discussed > upthread). Some parts applied down to 9.6 for the docs. Thanks. Here's the remainder, with some new ones. -- Justin >From ff7662fb2e68257bcffd9f0281220b5d3ab9dfbc Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 30 Mar 2020 19:43:22 -0500 Subject: [PATCH v9 01/15] doc: btree deduplication commit 0d861bbb702f8aa05c2a4e3f1650e7e8df8c8c27 Author: Peter Geoghegan --- doc/src/sgml/btree.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index 20cabe921f..bb395e6a85 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -642,7 +642,7 @@ options(relopts local_relopts *) returns Deduplication works by periodically merging groups of duplicate - tuples together, forming a single posting list tuple for each + tuples together, forming a single posting list tuple for each group. The column key value(s) only appear once in this representation. This is followed by a sorted array of TIDs that point to rows in the table. This -- 2.17.0 >From 9be911f869805b2862154d0170fa89fbd6be9e10 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:22:43 -0500 Subject: [PATCH v9 02/15] doc: pg_stat_progress_basebackup commit e65497df8f85ab9b9084c928ff69f384ea729b24 Author: Fujii Masao --- doc/src/sgml/monitoring.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 673a0e73e4..4e0193a967 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6089,8 +6089,8 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, waiting for checkpoint to finish The WAL sender process is currently performing - pg_start_backup to set up for - taking a base backup, and waiting for backup start + pg_start_backup to prepare to + take a base backup, and waiting for the start-of-backup checkpoint to finish. -- 2.17.0 >From 6500196ffbcc735eccb7623e728788aa3f5494bc Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 7 Apr 2020 19:56:56 -0500 Subject: [PATCH v9 03/15] doc: Allow users to limit storage reserved by replication slots commit c6550776394e25c1620bc8258427c8f1d448080d Author: Alvaro Herrera --- doc/src/sgml/config.sgml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 2c75876e32..6c1c9157d8 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3902,9 +3902,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows slots are allowed to retain in the pg_wal directory at checkpoint time. If max_slot_wal_keep_size is -1 (the default), -replication slots retain unlimited amount of WAL files. If -restart_lsn of a replication slot gets behind more than that megabytes -from the current LSN, the standby using the slot may no longer be able +replication slots may retain an unlimited amount of WAL files. Otherwise, if +restart_lsn of a replication slot falls behind the current LSN by more +than the given size, the standby using the slot may no longer be able to continue replication due to removal of required WAL files. You can see the WAL availability of replication slots in pg_replication_slots. -- 2.17.0 >From bf31db4f44e2d379660492a68343a1a65e10cb60 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 6 Apr 2020 17:16:07 -0500 Subject: [PATCH v9 04/15] doc: s/evade/avoid/ --- src/backend/access/gin/README | 2 +- src/backend/utils/adt/jsonpath_exec.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/access/gin/README b/src/backend/access/gin/README index 125a82219b..41d4e1e8a0 100644 --- a/src/backend/access/gin/README +++ b/src/backend/access/gin/README @@ -413,7 +413,7 @@ leftmost leaf of the tree. Deletion algorithm keeps exclusive locks on left siblings of pages comprising currently investigated path. Thus, if current page is to be removed, all required pages to remove both downlink and rightlink are already locked. That -evades potential right to left page locking order, which could deadlock with +avoids potential right to left page locking order, which could deadlock with concurrent stepping right. A search concurrent to page deletion might already have read a pointer to the diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c index 2c0b362502..0591c9effc 100644 ---
Re: doc review for v13
On Wed, Sep 09, 2020 at 09:37:42AM -0500, Justin Pryzby wrote: > I've added a few more. I have done an extra round of review on this patch series, and applied what looked obvious to me (basically the points already discussed upthread). Some parts applied down to 9.6 for the docs. -- Michael signature.asc Description: PGP signature
Re: doc review for v13
I've added a few more. -- Justin >From 137321a0d476f66b5e5f21c2f627c407330e50b1 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 30 Mar 2020 19:43:22 -0500 Subject: [PATCH v8 01/14] doc: btree deduplication commit 0d861bbb702f8aa05c2a4e3f1650e7e8df8c8c27 Author: Peter Geoghegan --- doc/src/sgml/btree.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index d03ee4d6fa..69c1ee0e97 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -642,7 +642,7 @@ options(relopts local_relopts *) returns Deduplication works by periodically merging groups of duplicate - tuples together, forming a single posting list tuple for each + tuples together, forming a single posting list tuple for each group. The column key value(s) only appear once in this representation. This is followed by a sorted array of TIDs that point to rows in the table. This -- 2.17.0 >From fc883d317a895140771ce34564847bb9ca98b7e3 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:22:43 -0500 Subject: [PATCH v8 02/14] doc: pg_stat_progress_basebackup commit e65497df8f85ab9b9084c928ff69f384ea729b24 Author: Fujii Masao --- doc/src/sgml/monitoring.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 304c49f07b..ea8780327f 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6069,8 +6069,8 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, waiting for checkpoint to finish The WAL sender process is currently performing - pg_start_backup to set up for - taking a base backup, and waiting for backup start + pg_start_backup to prepare to + take a base backup, and waiting for the start-of-backup checkpoint to finish. -- 2.17.0 >From 2d7c3ca43db77c910e8cda4b53fd6c6b09421b40 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 7 Apr 2020 19:56:56 -0500 Subject: [PATCH v8 03/14] doc: Allow users to limit storage reserved by replication slots commit c6550776394e25c1620bc8258427c8f1d448080d Author: Alvaro Herrera --- doc/src/sgml/config.sgml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 7a7177c550..190157df0a 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3900,9 +3900,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows slots are allowed to retain in the pg_wal directory at checkpoint time. If max_slot_wal_keep_size is -1 (the default), -replication slots retain unlimited amount of WAL files. If -restart_lsn of a replication slot gets behind more than that megabytes -from the current LSN, the standby using the slot may no longer be able +replication slots may retain an unlimited amount of WAL files. Otherwise, if +restart_lsn of a replication slot falls behind the current LSN by more +than the given size, the standby using the slot may no longer be able to continue replication due to removal of required WAL files. You can see the WAL availability of replication slots in pg_replication_slots. -- 2.17.0 >From e06f8c6f048daa9829dc3bf0450caf5c099d9e4f Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 6 Apr 2020 17:16:07 -0500 Subject: [PATCH v8 04/14] doc: s/evade/avoid/ --- src/backend/access/gin/README | 2 +- src/backend/utils/adt/jsonpath_exec.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/access/gin/README b/src/backend/access/gin/README index 125a82219b..41d4e1e8a0 100644 --- a/src/backend/access/gin/README +++ b/src/backend/access/gin/README @@ -413,7 +413,7 @@ leftmost leaf of the tree. Deletion algorithm keeps exclusive locks on left siblings of pages comprising currently investigated path. Thus, if current page is to be removed, all required pages to remove both downlink and rightlink are already locked. That -evades potential right to left page locking order, which could deadlock with +avoids potential right to left page locking order, which could deadlock with concurrent stepping right. A search concurrent to page deletion might already have read a pointer to the diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c index f146767bfc..2d2eb7d7a3 100644 --- a/src/backend/utils/adt/jsonpath_exec.c +++ b/src/backend/utils/adt/jsonpath_exec.c @@ -35,7 +35,7 @@ * executeItemOptUnwrapTarget() function have 'unwrap' argument, which indicates * whether unwrapping of array is needed. When unwrap == true, each of array * members is passed to executeItemOptUnwrapTarget() again but with unwrap == false - * in order to evade subsequent array unwrapping. + * in order to avoid
Re: doc review for v13
On Mon, Aug 31, 2020 at 08:42:08AM -0500, Justin Pryzby wrote: > On Mon, Aug 31, 2020 at 04:28:20PM +0900, Michael Paquier wrote: >> Wouldn't it be more simple to use "to prepare for a base backup" here? > > I think it's useful to say "prepare to take" since it's more specific.. It's > not "preparing to receive" or "preparing to scan" or "preparing to parse". Not sure I see the point in complicating the sentence here more than necessary. >>> - to have problems. Also, files which were ignored in the previous step >>> are >>> + to have problems. Files which were ignored in the previous step are >>> also ignored in this step. >> >> No sure this needs to change > > Two "also"s seems poor, and the first one detracts from the 2nd. Ah, OK. Indeed. >> "resent" is wrong, but "re-sent" does not sound like the best choice >> to me. Shouldn't we just say "sent again" for all three places? > > I don't think so. Well, using "sent again" has the advantage to about any ambiguity in the way it gets read. So I'd still prefer that when using the past tense of "send" in those sentences. Any opinions from others? -- Michael signature.asc Description: PGP signature
Re: doc review for v13
On Tue, Aug 18, 2020 at 12:17:03PM -0500, Justin Pryzby wrote: > The WAL sender process is currently performing > - pg_start_backup to set up for > - taking a base backup, and waiting for backup start > + pg_start_backup to prepare to > + take a base backup, and waiting for the start-of-backup > checkpoint to finish. Wouldn't it be more simple to use "to prepare for a base backup" here? > Run the dump in parallel by dumping class="parameter">njobs > -tables simultaneously. This option reduces the time of the dump but > it also > +tables simultaneously. This option may reduces the time needed to > perform the dump but it also > increases the load on the database server. You can only use this > option with the > [...] > Execute the reindex commands in parallel by running > njobs > -commands simultaneously. This option reduces the time of the > -processing but it also increases the load on the database server. > +commands simultaneously. This option may reduce the processing time > +but it also increases the load on the database server. > [...] > Execute the vacuum or analyze commands in parallel by running > njobs > -commands simultaneously. This option reduces the time of the > -processing but it also increases the load on the database server. > +commands simultaneously. This option may reduce the processing time > +but it also increases the load on the database server. > > > vacuumdb will open The original versions are fine IMO. > Replication is only supported by tables, including partitioned tables. > Attempts to replicate other types of relations such as views, > materialized > - views, or foreign tables, will result in an error. > + views, or foreign tables will result in an error. > I think that the original is fine. >* If the partition's attributes don't match the root relation's, we'll >* need to make a new attrmap which maps partition attribute numbers to > - * remoterel's, instead the original which maps root relation's > attribute > + * remoterel's, instead of the original which maps root relation's > attribute >* numbers to remoterel's. Indeed. >from the parent table will be created in the partition, if they don't >already exist. >If any of the CHECK constraints of the table being > - attached is marked NO INHERIT, the command will > fail; > + attached are marked NO INHERIT, the command will > fail; >such constraints must be recreated without the >NO INHERIT clause. Singular or plural depends on the context when if comes to any with a countable word, and plural looks more natural to me here. So, right. > enough for error messages. Detail and hint messages can be relegated to > a > verbose mode, or perhaps a pop-up error-details window. Also, details > and > hints would normally be suppressed from the server log to save > -space. Reference to implementation details is best avoided since users > -aren't expected to know the details. > +space. References to implementation details are best avoided since users > +aren't expected to know them. Original is fine IMO (see 6335c80). > not contain any checksums. Otherwise, it will contain a checksum > of each file in the backup using the specified algorithm. In > addition, > the manifest will always contain a SHA256 > -checksum of its own contents. The SHA algorithms > +checksum of its own content. The SHA algorithms > are significantly more CPU-intensive than CRC32C, > so selecting one of them may increase the time required to complete > the backup. > [...] > every check which will be performed by a running server when attempting > to make use of the backup. Even if you use this tool, you should still > perform test restores and verify that the resulting databases work as > - expected and that they appear to contain the correct data. However, > + expected and that they contain the correct data. However, > pg_verifybackup can detect many problems > that commonly occur due to storage problems or user error. > [...] > @@ -82,7 +82,7 @@ PostgreSQL documentation > for any files for which the computed checksum does not match the > checksum stored in the manifest. This step is not performed for any files > which produced errors in the previous step, since they are already known > - to have problems. Also, files which were ignored in the previous step are > + to have problems. Files which were ignored in the previous step are > also ignored in this step. No sure this needs to change > > > @@ -121,7 +121,7 @@ PostgreSQL documentation >Options > > > -The following
Re: doc review for v13
I stand by these changes which I proposed handful of times since April, but not yet included by Michael's previous commits. -- Justin >From f029efd79c4ad14ae003ed1a1c692931cdc33f1e Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 30 Mar 2020 19:43:22 -0500 Subject: [PATCH v6 01/10] doc: btree deduplication commit 0d861bbb702f8aa05c2a4e3f1650e7e8df8c8c27 Author: Peter Geoghegan --- doc/src/sgml/btree.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index d03ee4d6fa..69c1ee0e97 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -642,7 +642,7 @@ options(relopts local_relopts *) returns Deduplication works by periodically merging groups of duplicate - tuples together, forming a single posting list tuple for each + tuples together, forming a single posting list tuple for each group. The column key value(s) only appear once in this representation. This is followed by a sorted array of TIDs that point to rows in the table. This -- 2.17.0 >From aff29ab082f8915bd5fdfc9a763e23c864e555e0 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:22:43 -0500 Subject: [PATCH v6 02/10] doc: pg_stat_progress_basebackup commit e65497df8f85ab9b9084c928ff69f384ea729b24 Author: Fujii Masao --- doc/src/sgml/monitoring.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 304c49f07b..ea8780327f 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -6069,8 +6069,8 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, waiting for checkpoint to finish The WAL sender process is currently performing - pg_start_backup to set up for - taking a base backup, and waiting for backup start + pg_start_backup to prepare to + take a base backup, and waiting for the start-of-backup checkpoint to finish. -- 2.17.0 >From 0949708420623878995ef82d26fed2348edad941 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 5 Apr 2020 22:35:09 -0500 Subject: [PATCH v6 03/10] Fix docs: "time of the" commit 9e257a181cc1dc5e19eb5d770ce09cc98f470f5f Author: Andrew Dunstan Date: Sun Mar 24 11:27:20 2013 -0400 Add parallel pg_dump option. commit 5ab892c391c6bc54a00e7a8de5cab077cabace6a Author: Michael Paquier Date: Sat Jul 27 22:21:18 2019 +0900 Add support for --jobs in reindexdb commit a17923204736d8842eade3517d6a8ee81290fca4 Author: Alvaro Herrera Date: Fri Jan 23 15:02:45 2015 -0300 vacuumdb: enable parallel mode --- doc/src/sgml/ref/pg_dump.sgml | 2 +- doc/src/sgml/ref/reindexdb.sgml | 4 ++-- doc/src/sgml/ref/vacuumdb.sgml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index 7a37fd8045..77029defc5 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -322,7 +322,7 @@ PostgreSQL documentation Run the dump in parallel by dumping njobs -tables simultaneously. This option reduces the time of the dump but it also +tables simultaneously. This option may reduces the time needed to perform the dump but it also increases the load on the database server. You can only use this option with the directory output format because this is the only output format where multiple processes can write their data at the same time. diff --git a/doc/src/sgml/ref/reindexdb.sgml b/doc/src/sgml/ref/reindexdb.sgml index 026fd018d9..4f821c2095 100644 --- a/doc/src/sgml/ref/reindexdb.sgml +++ b/doc/src/sgml/ref/reindexdb.sgml @@ -174,8 +174,8 @@ PostgreSQL documentation Execute the reindex commands in parallel by running njobs -commands simultaneously. This option reduces the time of the -processing but it also increases the load on the database server. +commands simultaneously. This option may reduce the processing time +but it also increases the load on the database server. reindexdb will open diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml index 95d6894cb0..7b13950552 100644 --- a/doc/src/sgml/ref/vacuumdb.sgml +++ b/doc/src/sgml/ref/vacuumdb.sgml @@ -155,8 +155,8 @@ PostgreSQL documentation Execute the vacuum or analyze commands in parallel by running njobs -commands simultaneously. This option reduces the time of the -processing but it also increases the load on the database server. +commands simultaneously. This option may reduce the processing time +but it also increases the load on the database server. vacuumdb will open -- 2.17.0 >From 0457a0038e2c4fc473dbdbbeb3536426b5939bcd Mon Sep 17 00:00:00 2001 From: Justin Pryzby
Re: doc review for v13
On Fri, Jun 12, 2020 at 09:13:02PM +0900, Michael Paquier wrote: > I have merged 0003 and 0004 together and applied them. 0005 seems to > have a separate issue as mentioned upthread, and I have not really > looked at 0001 and 0002. Thanks. And committed 0001 and 0002 after some tiny adjustments as of 7a3543c. -- Michael signature.asc Description: PGP signature
Re: doc review for v13
On Thu, Jun 11, 2020 at 09:37:09PM -0500, Justin Pryzby wrote: > Some new bits, > And some old ones. I have merged 0003 and 0004 together and applied them. 0005 seems to have a separate issue as mentioned upthread, and I have not really looked at 0001 and 0002. Thanks. -- Michael signature.asc Description: PGP signature
Re: doc review for v13
On Thu, Jun 11, 2020 at 09:37:09PM -0500, Justin Pryzby wrote: > Some new bits, > And some old ones. I was looking at this patch set, and 0005 has attracted my attention here: > --- a/src/backend/utils/cache/relcache.c > +++ b/src/backend/utils/cache/relcache.c > @@ -4240,7 +4240,6 @@ AttrDefaultFetch(Relation relation) > HeapTuple htup; > Datum val; > boolisnull; > - int found; > int i; Since 16828d5, this variable is indeed unused. Now, the same commit has removed the following code: - if (found != ndef) - elog(WARNING, "%d attrdef record(s) missing for rel %s", -ndef - found, RelationGetRelationName(relation)); Should we actually keep this variable and have this sanity check in place? It seems to me that it would be good to have that, so as we can make sure that the number of default attributes cached matches with the number of defaults actually found when scanning each attribute. Adding in CC Andrew as the author of 16828d5 for more input. -- Michael signature.asc Description: PGP signature
Re: doc review for v13
Some new bits,
And some old ones.
--
Justin
>From b74bef6f9d36a2fac71045ab707ed4be65730ba7 Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Wed, 29 Apr 2020 13:13:29 -0500
Subject: [PATCH v5 1/5] Fix comments for WITH OIDs, removed at 578b22971
Previously mentioned here:
https://www.postgresql.org/message-id/20200429194622.ga25...@telsasoft.com
---
src/backend/access/common/tupdesc.c | 4 +---
src/backend/access/transam/varsup.c | 3 +--
src/backend/commands/tablecmds.c| 5 ++---
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 1e743d7d86..ce84e22cbd 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -786,9 +786,7 @@ TupleDescInitEntryCollation(TupleDesc desc,
*
* Given a relation schema (list of ColumnDef nodes), build a TupleDesc.
*
- * Note: the default assumption is no OIDs; caller may modify the returned
- * TupleDesc if it wants OIDs. Also, tdtypeid will need to be filled in
- * later on.
+ * tdtypeid will need to be filled in later on.
*/
TupleDesc
BuildDescForRelation(List *schema)
diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c
index e14b53bf9e..8328b8050a 100644
--- a/src/backend/access/transam/varsup.c
+++ b/src/backend/access/transam/varsup.c
@@ -527,8 +527,7 @@ GetNewObjectId(void)
* The first time through this routine after normal postmaster start, the
* counter will be forced up to FirstNormalObjectId. This mechanism
* leaves the OIDs between FirstBootstrapObjectId and FirstNormalObjectId
- * available for automatic assignment during initdb, while ensuring they
- * will never conflict with user-assigned OIDs.
+ * available for automatic assignment during initdb.
*/
if (ShmemVariableCache->nextOid < ((Oid) FirstNormalObjectId))
{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2ab02e01a0..35945bce73 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4829,7 +4829,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
continue;
/*
- * If we change column data types or add/remove OIDs, the operation
+ * If we change column data types, the operation
* has to be propagated to tables that use this table's rowtype as a
* column type. tab->newvals will also be non-NULL in the case where
* we're adding a column with a default. We choose to forbid that
@@ -4853,8 +4853,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
/*
* We only need to rewrite the table if at least one column needs to
- * be recomputed, we are adding/removing the OID column, or we are
- * changing its persistence.
+ * be recomputed, or we are changing its persistence.
*
* There are two reasons for requiring a rewrite when changing
* persistence: on one hand, we need to ensure that the buffers
--
2.17.0
>From eb85fabed31d7c3a8fa1396ddfd2ea733c5c8baf Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 10 May 2020 17:30:14 -0500
Subject: [PATCH v5 2/5] Remove mention of double timestamps..
..which were removed in v10.
This was previously discussed here:
https://www.postgresql.org/message-id/20191230074721.GM12890%40telsasoft.com
---
src/backend/utils/adt/selfuncs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 0f02f32ffd..be08eb4814 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4018,8 +4018,8 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel,
* to be treated separately.
*
* The several datatypes representing absolute times are all converted
- * to Timestamp, which is actually a double, and then we just use that
- * double value. Note this will give correct results even for the "special"
+ * to Timestamp, which is actually an int64, and then we promote that to
+ * a double. Note this will give correct results even for the "special"
* values of Timestamp, since those are chosen to compare correctly;
* see timestamp_cmp.
*
--
2.17.0
>From c0501535b1ec3709abb8334ddd29f2603bece5ca Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 3 May 2020 19:57:32 -0500
Subject: [PATCH v5 3/5] possessive: its (and more)
---
src/backend/replication/logical/origin.c | 2 +-
src/backend/utils/mmgr/README| 2 +-
src/bin/pg_dump/pg_backup_archiver.c | 6 +++---
src/bin/pg_dump/pg_backup_custom.c | 8 +++-
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 1ca4479605..dec9e95119 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -427,7 +427,7 @@ restart:
/*
- * Lookup replication
Re: doc review for v13
On Mon, Apr 27, 2020 at 03:03:05PM +0900, Michael Paquier wrote: > Hm, okay. There are still pieces in those patches about which I am > not sure, so I have let that aside for the time being. > > Anyway, I have applied patch 12, and reported the typos from imath.c Thank you. I will leave this here in case someone else wants to make a pass or vet them. diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml index 192d6574c3..de2be61bff 100644 --- a/doc/src/sgml/auto-explain.sgml +++ b/doc/src/sgml/auto-explain.sgml @@ -200,9 +200,9 @@ LOAD 'auto_explain'; auto_explain.log_settings controls whether information - about modified configuration options is printed when execution plan is logged. - Only options affecting query planning with value different from the built-in - default value are included in the output. This parameter is off by default. + about modified configuration options is printed when an execution plan is logged. + Only those options which affect query planning and whose value differs from their + built-in default are included in the output. This parameter is off by default. Only superusers can change this setting. diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index e9cab4a55d..ff1e49e509 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -609,7 +609,7 @@ equalimage(opcintype oid) returns bool Deduplication works by periodically merging groups of duplicate - tuples together, forming a single posting list tuple for each + tuples together, forming a single posting list tuple for each group. The column key value(s) only appear once in this representation. This is followed by a sorted array of TIDs that point to rows in the table. This diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index a14df06292..87e0183a89 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3667,7 +3667,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows servers or streaming base backup clients (i.e., the maximum number of simultaneously running WAL sender processes). The default is 10. The value 0 means -replication is disabled. Abrupt streaming client disconnection might +replication is disabled. Abrupt disconnection of a streaming client might leave an orphaned connection slot behind until a timeout is reached, so this parameter should be set slightly higher than the maximum number of expected clients so disconnected clients can immediately @@ -3790,9 +3790,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows slots are allowed to retain in the pg_wal directory at checkpoint time. If max_slot_wal_keep_size is -1 (the default), -replication slots retain unlimited amount of WAL files. If -restart_lsn of a replication slot gets behind more than that megabytes -from the current LSN, the standby using the slot may no longer be able +replication slots retain unlimited amount of WAL files. Otherwise, if +restart_lsn of a replication slot falls behind the current LSN by more +than the specified size, the standby using the slot may no longer be able to continue replication due to removal of required WAL files. You can see the WAL availability of replication slots in pg_replication_slots. diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 2a478c4f73..9ec0d4c783 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -3959,7 +3959,7 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02 Before running the ATTACH PARTITION command, it is recommended to create a CHECK constraint on the table to be attached matching the desired partition constraint. That way, - the system will be able to skip the scan to validate the implicit + the system will be able to skip the scan which is otherwise needed to validate the implicit partition constraint. Without the CHECK constraint, the table will be scanned to validate the partition constraint while holding an ACCESS EXCLUSIVE lock on that partition diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 75d2224a61..b9c789eb6f 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -925,11 +925,11 @@ postgresql:///mydb?host=localhostport=5433 -Connection URI needs to be encoded with +A connection URI needs to be encoded with https://tools.ietf.org/html/rfc3986#section-2.1;>Percent-encoding -if it includes symbols with special meaning in any of its parts. -Here is an example where equal sign (=) is replaced -with %3D and whitespace character with +if it includes symbols with special meanings in any of its parts. +Here
Re: doc review for v13
On Sun, Apr 26, 2020 at 08:59:05PM -0400, Tom Lane wrote: > James Coleman writes: >> On Sun, Apr 26, 2020 at 12:13 PM Justin Pryzby wrote: >>> I think my text is correct. This would *also* be correct: >>> | If any CHECK constraint on the table being >>> | attached is marked NO INHERIT, the command will >>> fail; >>> But not the hybrid: "If any OF THE .. is .." > >> "any of the...are" sounds more natural to my ears, > > Yeah, I think the same. If you want to argue grammar, I'd point > out that the "any" could refer to several of the constraints, > making it correct to use the plural verb. The alternative that > Justin mentions could be written as "If any one constraint is ...", > which is correct in that form; but the plural way seems less stilted. Hm, okay. There are still pieces in those patches about which I am not sure, so I have let that aside for the time being. Anyway, I have applied patch 12, and reported the typos from imath.c directly to upstream: https://github.com/creachadair/imath/issues/45 https://github.com/creachadair/imath/issues/46 -- Michael signature.asc Description: PGP signature
Re: doc review for v13
James Coleman writes: > On Sun, Apr 26, 2020 at 12:13 PM Justin Pryzby wrote: >> I think my text is correct. This would *also* be correct: >> | If any CHECK constraint on the table being >> | attached is marked NO INHERIT, the command will >> fail; >> But not the hybrid: "If any OF THE .. is .." > "any of the...are" sounds more natural to my ears, Yeah, I think the same. If you want to argue grammar, I'd point out that the "any" could refer to several of the constraints, making it correct to use the plural verb. The alternative that Justin mentions could be written as "If any one constraint is ...", which is correct in that form; but the plural way seems less stilted. regards, tom lane
Re: doc review for v13
On Sun, Apr 26, 2020 at 12:13 PM Justin Pryzby wrote: > > On Tue, Apr 14, 2020 at 02:47:54PM +0900, Michael Paquier wrote: > > On Sun, Apr 12, 2020 at 04:35:45PM -0500, Justin Pryzby wrote: > > > Added a few more. > > > And rebased on top of dbc60c5593f26dc777a3be032bff4fb4eab1ddd1 > > > > Thanks for the patch set, I have applied the most obvious parts (more > > or less 1/3) to reduce the load. Here is a review of the rest. > > Thanks - attached are the remaining undisputed portions.. > > > > +++ b/doc/src/sgml/ref/alter_table.sgml > > > @@ -889,7 +889,7 @@ WITH ( MODULUS > > class="parameter">numeric_literal, REM > > >from the parent table will be created in the partition, if they > > > don't > > >already exist. > > >If any of the CHECK constraints of the table > > > being > > > - attached is marked NO INHERIT, the command will > > > fail; > > > + attached are marked NO INHERIT, the command > > > will fail; > > >such constraints must be recreated without the > > >NO INHERIT clause. > > > > > > > It seems to me that both are actually correct here. > > I think my text is correct. This would *also* be correct: > > | If any CHECK constraint on the table being > | attached is marked NO INHERIT, the command will > fail; > > But not the hybrid: "If any OF THE .. is .." "any of the...are" sounds more natural to my ears, and some searching yielded some grammar sites that agree (specifically that "any of" is only used with singular verbs if the construction is uncountable or negative). However there are also multiple claims by grammarians that either singular or plural verbs are acceptable with the "any of" construction. So that's not all that helpful. James
Re: doc review for v13
On Tue, Apr 14, 2020 at 02:47:54PM +0900, Michael Paquier wrote: > On Sun, Apr 12, 2020 at 04:35:45PM -0500, Justin Pryzby wrote: > > Added a few more. > > And rebased on top of dbc60c5593f26dc777a3be032bff4fb4eab1ddd1 > > Thanks for the patch set, I have applied the most obvious parts (more > or less 1/3) to reduce the load. Here is a review of the rest. Thanks - attached are the remaining undisputed portions.. > > +++ b/doc/src/sgml/ref/alter_table.sgml > > @@ -889,7 +889,7 @@ WITH ( MODULUS > class="parameter">numeric_literal, REM > >from the parent table will be created in the partition, if they don't > >already exist. > >If any of the CHECK constraints of the table being > > - attached is marked NO INHERIT, the command will > > fail; > > + attached are marked NO INHERIT, the command will > > fail; > >such constraints must be recreated without the > >NO INHERIT clause. > > > > It seems to me that both are actually correct here. I think my text is correct. This would *also* be correct: | If any CHECK constraint on the table being | attached is marked NO INHERIT, the command will fail; But not the hybrid: "If any OF THE .. is .." -- Justin >From 5f499fec02301696705be5b9f6e1b0c4fa1dba16 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:31:08 -0500 Subject: [PATCH v4 01/12] doc: percent encoding commit e0ed6817c0ee218a3681920807404603e042ff04 Author: Michael Paquier --- doc/src/sgml/libpq.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 75d2224a61..8b9af95c14 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -925,11 +925,11 @@ postgresql:///mydb?host=localhostport=5433 -Connection URI needs to be encoded with +A connection URI needs to be encoded with https://tools.ietf.org/html/rfc3986#section-2.1;>Percent-encoding -if it includes symbols with special meaning in any of its parts. -Here is an example where equal sign (=) is replaced -with %3D and whitespace character with +if it includes symbols with special meanings in any of its parts. +Here is an example where an equal sign (=) is replaced +with %3D and a whitespace character with %20: postgresql://user@localhost:5433/mydb?options=-c%20synchronous_commit%3Doff -- 2.17.0 >From 1df7e0445624948b63d776f6894da145db8622cc Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 30 Mar 2020 19:43:22 -0500 Subject: [PATCH v4 02/12] docs: btree deduplication commit 0d861bbb702f8aa05c2a4e3f1650e7e8df8c8c27 Author: Peter Geoghegan --- doc/src/sgml/btree.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index e9cab4a55d..ff1e49e509 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -609,7 +609,7 @@ equalimage(opcintype oid) returns bool Deduplication works by periodically merging groups of duplicate - tuples together, forming a single posting list tuple for each + tuples together, forming a single posting list tuple for each group. The column key value(s) only appear once in this representation. This is followed by a sorted array of TIDs that point to rows in the table. This -- 2.17.0 >From 383aabd1e2fe497f40710904d897f088607e08a2 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:22:43 -0500 Subject: [PATCH v4 03/12] doc: pg_stat_progress_basebackup commit e65497df8f85ab9b9084c928ff69f384ea729b24 Author: Fujii Masao --- doc/src/sgml/monitoring.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 6562cc400b..d5d9da659d 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -4577,8 +4577,8 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, waiting for checkpoint to finish The WAL sender process is currently performing - pg_start_backup to set up for - taking a base backup, and waiting for backup start + pg_start_backup to prepare to + take a base backup, and waiting for the start-of-backup checkpoint to finish. -- 2.17.0 >From 44cc3b9422619ff068e72fbad3f0eec103594717 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 5 Apr 2020 22:35:09 -0500 Subject: [PATCH v4 04/12] Fix docs: "time of the" commit 9e257a181cc1dc5e19eb5d770ce09cc98f470f5f Author: Andrew Dunstan Date: Sun Mar 24 11:27:20 2013 -0400 Add parallel pg_dump option. commit 5ab892c391c6bc54a00e7a8de5cab077cabace6a Author: Michael Paquier Date: Sat Jul 27 22:21:18 2019 +0900 Add support for --jobs in reindexdb commit a17923204736d8842eade3517d6a8ee81290fca4 Author: Alvaro Herrera Date: Fri Jan 23 15:02:45 2015 -0300 vacuumdb: enable parallel mode ---
Re: doc review for v13
On Sun, Apr 12, 2020 at 04:35:45PM -0500, Justin Pryzby wrote:
> Added a few more.
> And rebased on top of dbc60c5593f26dc777a3be032bff4fb4eab1ddd1
Thanks for the patch set, I have applied the most obvious parts (more
or less 1/3) to reduce the load. Here is a review of the rest.
> @@ -2829,7 +2829,6 @@
> show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
>
> ExplainPropertyList("Sort Methods Used", methodNames, es);
>
> - if (groupInfo->maxMemorySpaceUsed > 0)
> {
> longavgSpace =
> groupInfo->totalMemorySpaceUsed / groupInfo->groupCount;
> const char *spaceTypeName;
> @@ -2846,7 +2845,7 @@
> show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
>
> ExplainCloseGroup("Sort Spaces", memoryName.data, true,
> es);
> }
> - if (groupInfo->maxDiskSpaceUsed > 0)
> +
> {
> longavgSpace =
> groupInfo->totalDiskSpaceUsed / groupInfo->groupCount;
> const char *spaceTypeName;
If this can be reworked, it seems to me that more cleanup could be
done.
> @@ -987,7 +987,7 @@ ExecInitIncrementalSort(IncrementalSort *node, EState
> *estate, int eflags)
>
> /*
>* Incremental sort can't be used with either EXEC_FLAG_REWIND,
> - * EXEC_FLAG_BACKWARD or EXEC_FLAG_MARK, because we only one of many
> sort
> + * EXEC_FLAG_BACKWARD or EXEC_FLAG_MARK, because we only one of
> many sort
>* batches in the current sort state.
>*/
> Assert((eflags & (EXEC_FLAG_BACKWARD |
The following is inconsistent with this comment block, and I guess
that "" should be "have":
Assert((eflags & (EXEC_FLAG_BACKWARD |
EXEC_FLAG_MARK)) == 0);
This is only a doc-related change though, so I'll start a different
thread about that after looking more at it.
> @@ -1153,7 +1153,7 @@ ExecReScanIncrementalSort(IncrementalSortState *node)
> /*
>* If we've set up either of the sort states yet, we need to reset them.
>* We could end them and null out the pointers, but there's no reason to
> - * repay the setup cost, and because guard setting up pivot comparator
> + * repay the setup cost, and because guard setting up pivot
> comparator
>* state similarly, doing so might actually cause a leak.
>*/
> if (node->fullsort_state != NULL)
I don't quite understand this comment either, but it seems to me that
the last part should be a fully-separate sentence, aka "This guards
against..".
> @@ -631,7 +631,7 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
> /*
>* If the partition's attributes don't match the root relation's, we'll
>* need to make a new attrmap which maps partition attribute numbers to
> - * remoterel's, instead the original which maps root relation's
> attribute
> + * remoterel's, instead of the original which maps root relation's
> attribute
>* numbers to remoterel's.
>*
>* Note that 'map' which comes from the tuple routing data structure
Okay, this is not really clear to start with. I think that I would
rewrite that completely as follows:
"If the partition's attributes do not match the root relation's
attributes, we cannot use the original attribute map which maps the
root relation's attributes with remoterel's attributes. Instead,
build a new attribute map which maps the partition's attributes with
remoterel's attributes."
> +++ b/src/backend/storage/lmgr/proc.c
> @@ -1373,7 +1373,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod
> lockMethodTable)
> else
> LWLockRelease(ProcArrayLock);
>
> - /* prevent signal from being resent more than once */
> + /* prevent signal from being re-sent more than once */
> allow_autovacuum_cancel = false;
> }
Shouldn't that just be "sent more than two times"?
> @@ -1428,11 +1428,11 @@ tuplesort_updatemax(Tuplesortstate *state)
> }
>
> /*
> - * Sort evicts data to the disk when it didn't manage to fit those data
> to
> - * the main memory. This is why we assume space used on the disk to be
> + * Sort evicts data to the disk when it didn't manage to fit the data in
> + * main memory. This is why we assume space used on the disk to be
Both the original and the suggestion are wrong? It seems to me that
it should be "this data" instead because it refers to the data evicted
in the first part of the sentence.
>* more important for tracking resource usage than space used in memory.
> - * Note that amount of space occupied by some tuple set on the disk
> might
> - * be less than amount of space occupied by the same tuple set in the
> + * Note that amount of
Re: doc review for v13
Added a few more. And rebased on top of dbc60c5593f26dc777a3be032bff4fb4eab1ddd1 -- Justin >From 37b796862eb8c08dbe0d1a6947a8212d1c515491 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 19:51:32 -0500 Subject: [PATCH v3 01/20] doc: psql opclass/opfamily commit b0b5e20cd8d1a58a8782d5dc806a5232db116e2f Author: Alexander Korotkov --- doc/src/sgml/ref/psql-ref.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 0595d1c04b..cdd24fad98 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1244,7 +1244,7 @@ testdb= (see ). If access-method-patttern is specified, only operator classes associated with access methods whose -names match pattern are listed. +names match the pattern are listed. If input-type-pattern is specified, only operator classes associated with input types whose names match the pattern are listed. @@ -1267,7 +1267,7 @@ testdb= (see ). If access-method-patttern is specified, only operator families associated with access methods whose -names match pattern are listed. +names match the pattern are listed. If input-type-pattern is specified, only operator families associated with input types whose names match the pattern are listed. @@ -1291,7 +1291,7 @@ testdb= (). If access-method-patttern is specified, only members of operator families associated with access -methods whose names match pattern are listed. +methods whose names match the pattern are listed. If input-type-pattern is specified, only members of operator families whose names match the pattern are listed. @@ -1314,7 +1314,7 @@ testdb= (). If access-method-patttern is specified, only members of operator families associated with access -methods whose names match pattern are listed. +methods whose names match the pattern are listed. If input-type-pattern is specified, only members of operator families whose names match the pattern are listed. -- 2.17.0 >From 05526ece41acad552697753a156f161a4253c831 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:31:08 -0500 Subject: [PATCH v3 02/20] doc: percent encoding commit e0ed6817c0ee218a3681920807404603e042ff04 Author: Michael Paquier --- doc/src/sgml/libpq.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 75d2224a61..8b9af95c14 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -925,11 +925,11 @@ postgresql:///mydb?host=localhostport=5433 -Connection URI needs to be encoded with +A connection URI needs to be encoded with https://tools.ietf.org/html/rfc3986#section-2.1;>Percent-encoding -if it includes symbols with special meaning in any of its parts. -Here is an example where equal sign (=) is replaced -with %3D and whitespace character with +if it includes symbols with special meanings in any of its parts. +Here is an example where an equal sign (=) is replaced +with %3D and a whitespace character with %20: postgresql://user@localhost:5433/mydb?options=-c%20synchronous_commit%3Doff -- 2.17.0 >From 290c309de4ce3ac3103cd527406c75165db4c4e0 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 30 Mar 2020 19:43:22 -0500 Subject: [PATCH v3 03/20] docs: btree deduplication commit 0d861bbb702f8aa05c2a4e3f1650e7e8df8c8c27 Author: Peter Geoghegan --- doc/src/sgml/btree.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index e9cab4a55d..ff1e49e509 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -609,7 +609,7 @@ equalimage(opcintype oid) returns bool Deduplication works by periodically merging groups of duplicate - tuples together, forming a single posting list tuple for each + tuples together, forming a single posting list tuple for each group. The column key value(s) only appear once in this representation. This is followed by a sorted array of TIDs that point to rows in the table. This -- 2.17.0 >From d683b732ab33b3d7f3664a870ad52ca78f44041f Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:22:43 -0500 Subject: [PATCH v3 04/20] doc: pg_stat_progress_basebackup commit e65497df8f85ab9b9084c928ff69f384ea729b24 Author: Fujii Masao --- doc/src/sgml/monitoring.sgml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index c50b72137f..3ad1fd7005 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -4483,7 +4483,7 @@ SELECT
Re: doc review for v13
On Thu, Apr 09, 2020 at 10:01:51PM -0500, Justin Pryzby wrote: > On Fri, Apr 10, 2020 at 11:27:46AM +0900, Michael Paquier wrote: >> required pages to remove both downlink and rightlink are already locked. >> That >> -evades potential right to left page locking order, which could deadlock with >> +avoid potential right to left page locking order, which could deadlock with >> Not sure which one is better, but the new change is grammatically >> incorrect. > > "Evades" usually means to act to avoid detection by the government. Like tax > evasion. This change is from Alexander Korotkov as of 32ca32d, so I am adding him in CC to get his opinion. >>auto_explain.log_settings controls whether >> information >> - about modified configuration options are printed when execution plan >> is logged. >> - Only options affecting query planning with value different from the >> built-in >> + about modified configuration options is printed when an execution >> plan is logged. >> + Only those options which affect query planning and whose value >> differs from its built-in >> Depends on how you read the sentence, but here is seemt to me that >> "statistics" is the correct subject, no? > > Statistics ? Oops. I may have messed up with a different part of the patch set. Your suggestion is right as the subject is "information" here. -- Michael signature.asc Description: PGP signature
Re: doc review for v13
On Fri, Apr 10, 2020 at 11:27:46AM +0900, Michael Paquier wrote: > On Wed, Apr 08, 2020 at 11:56:53AM -0500, Justin Pryzby wrote: > > I previously mailed separately about a few individual patches, some of which > > have separate, ongoing discussion and aren't included here (incr sort, > > parallel > > vacuum). > > I have gone through your changes, and committed what looked like the > most obvious mistakes in my view. Please see below for more > comments. Thanks - rebased for cfbot and continued review. > required pages to remove both downlink and rightlink are already locked. > That > -evades potential right to left page locking order, which could deadlock with > +avoid potential right to left page locking order, which could deadlock with > Not sure which one is better, but the new change is grammatically > incorrect. Thanks for noticing "Evades" usually means to act to avoid detection by the government. Like tax evasion. >auto_explain.log_settings controls whether > information > - about modified configuration options are printed when execution plan > is logged. > - Only options affecting query planning with value different from the > built-in > + about modified configuration options is printed when an execution plan > is logged. > + Only those options which affect query planning and whose value differs > from its built-in > Depends on how you read the sentence, but here is seemt to me that > "statistics" is the correct subject, no? Statistics ? > _tz suffix. These functions have been implemented to > -support comparison of date/time values that involves implicit > +support comparison of date/time values that involve implicit > The subject is "comparison" here, no? You're right. -- Justin >From e18b7dbdb3b75047540691929a7ef2b55c0b58e2 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 19:51:32 -0500 Subject: [PATCH v2 01/12] doc: psql opclass/opfamily commit b0b5e20cd8d1a58a8782d5dc806a5232db116e2f Author: Alexander Korotkov ALSO, should we rename the "Purpose" column? I see we have pg_amop.amoppurpose so maybe it's fine ? --- doc/src/sgml/ref/psql-ref.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 0595d1c04b..cdd24fad98 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1244,7 +1244,7 @@ testdb= (see ). If access-method-patttern is specified, only operator classes associated with access methods whose -names match pattern are listed. +names match the pattern are listed. If input-type-pattern is specified, only operator classes associated with input types whose names match the pattern are listed. @@ -1267,7 +1267,7 @@ testdb= (see ). If access-method-patttern is specified, only operator families associated with access methods whose -names match pattern are listed. +names match the pattern are listed. If input-type-pattern is specified, only operator families associated with input types whose names match the pattern are listed. @@ -1291,7 +1291,7 @@ testdb= (). If access-method-patttern is specified, only members of operator families associated with access -methods whose names match pattern are listed. +methods whose names match the pattern are listed. If input-type-pattern is specified, only members of operator families whose names match the pattern are listed. @@ -1314,7 +1314,7 @@ testdb= (). If access-method-patttern is specified, only members of operator families associated with access -methods whose names match pattern are listed. +methods whose names match the pattern are listed. If input-type-pattern is specified, only members of operator families whose names match the pattern are listed. -- 2.17.0 >From 650c29ca5ce500337bd0ff7345f9cfcb60dd596a Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Mar 2020 21:31:08 -0500 Subject: [PATCH v2 02/12] doc: percent encoding commit e0ed6817c0ee218a3681920807404603e042ff04 Author: Michael Paquier --- doc/src/sgml/libpq.sgml | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 75d2224a61..8b9af95c14 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -925,11 +925,11 @@ postgresql:///mydb?host=localhostport=5433 -Connection URI needs to be encoded with +A connection URI needs to be encoded with https://tools.ietf.org/html/rfc3986#section-2.1;>Percent-encoding -if it includes symbols with special meaning in any of its parts. -Here is an example where equal sign (=) is replaced -with %3D and whitespace character with
Re: doc review for v13
On Wed, Apr 08, 2020 at 11:56:53AM -0500, Justin Pryzby wrote: > I previously mailed separately about a few individual patches, some of which > have separate, ongoing discussion and aren't included here (incr sort, > parallel > vacuum). I have gone through your changes, and committed what looked like the most obvious mistakes in my view. Please see below for more comments. required pages to remove both downlink and rightlink are already locked. That -evades potential right to left page locking order, which could deadlock with +avoid potential right to left page locking order, which could deadlock with Not sure which one is better, but the new change is grammatically incorrect. auto_explain.log_settings controls whether information - about modified configuration options are printed when execution plan is logged. - Only options affecting query planning with value different from the built-in + about modified configuration options is printed when an execution plan is logged. + Only those options which affect query planning and whose value differs from its built-in Depends on how you read the sentence, but here is seemt to me that "statistics" is the correct subject, no? -replication is disabled. Abrupt streaming client disconnection might +replication is disabled. Abrupt disconnection of a streaming client might Original looks correct to me here. _tz suffix. These functions have been implemented to -support comparison of date/time values that involves implicit +support comparison of date/time values that involve implicit The subject is "comparison" here, no? may be included. It also stores the size, last modification time, and -an optional checksum for each file. +optionally a checksum for each file. The original sounds fine to me as well. Anything related to imath.c needs to be reported upstream, though I recall reporting these two already. -- Michael signature.asc Description: PGP signature
doc review for v13
I reviewed docs for v13, like:
git log --cherry-pick origin/master...origin/REL_12_STABLE -p doc
I did something similar for v12 [0]. I've included portions of that here which
still seem lacking 12 months later (but I'm not intending to continue defending
each individual patch hunk).
I previously mailed separately about a few individual patches, some of which
have separate, ongoing discussion and aren't included here (incr sort, parallel
vacuum).
Justin
[0]
https://www.postgresql.org/message-id/flat/20190709161256.GH22387%40telsasoft.com#56889b868e5886e36b90e9f5a1165186
>From 482b590355cd7df327602dd36e91721b827f9c37 Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 29 Mar 2020 19:31:04 -0500
Subject: [PATCH v1 01/19] docs: pg_statistic_ext.stxstattarget
commit c31132d87c6315bbbe4b4aa383705aaae2348c0e
Author: Tomas Vondra
Date: Wed Mar 18 16:48:12 2020 +0100
---
doc/src/sgml/catalogs.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 386c6d7bd1..ce33df9e58 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6472,7 +6472,7 @@ SCRAM-SHA-256$iteration count:
.
A zero value indicates that no statistics should be collected.
A negative value says to use the system default statistics target.
- Positive values stxstattarget
+ Positive values of stxstattarget
determine the target number of most common values
to collect.
--
2.17.0
>From 2a3a4d7028b02070447fafd37e66e72da59966bf Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 29 Mar 2020 19:33:44 -0500
Subject: [PATCH v1 02/19] docs: reg* functions
commit 8408e3a557ad26a7e88f867a425b2b9a86c4fa04
Author: Peter Eisentraut
Date: Wed Mar 18 14:51:37 2020 +0100
---
doc/src/sgml/func.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a38387b8c6..fd0f5d64b3 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18796,7 +18796,7 @@ SELECT collation for ('foo' COLLATE "de_DE");
to_regnamespace, to_regoper,
to_regoperator, to_regrole,
to_regproc, to_regprocedure, and
- to_regtype, functions translate relation, collation, schema,
+ to_regtype translate relation, collation, schema,
operator, role, function, and type names (given as text) to
objects of the corresponding reg* type (see about the types). These functions differ from a
--
2.17.0
>From 6864ced0a9eaeab4c010d1f090b26b337f125742 Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 29 Mar 2020 19:43:42 -0500
Subject: [PATCH v1 03/19] Minus one
See also
ac862376037727e744f25030bd8b6090c707247b
---
doc/src/sgml/config.sgml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a0da4aabac..ea2749535d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6110,7 +6110,7 @@ local0.*/var/log/postgresql
unoptimized queries in your applications.
If this value is specified without units, it is taken as milliseconds.
Setting this to zero prints all statement durations.
- Minus-one (the default) disables logging statement durations.
+ -1 (the default) disables logging statement durations.
Only superusers can change this setting.
@@ -6162,7 +6162,7 @@ local0.*/var/log/postgresql
traffic is too high to log all queries.
If this value is specified without units, it is taken as milliseconds.
Setting this to zero samples all statement durations.
- Minus-one (the default) disables sampling statement durations.
+ -1 (the default) disables sampling statement durations.
Only superusers can change this setting.
--
2.17.0
>From bfb8439eb5618db3a36ca2794dbcc35489d98c27 Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 29 Mar 2020 19:51:32 -0500
Subject: [PATCH v1 04/19] doc: psql opclass/opfamily
commit b0b5e20cd8d1a58a8782d5dc806a5232db116e2f
Author: Alexander Korotkov
ALSO, should we rename the "Purpose" column? I see we have pg_amop.amoppurpose
so maybe it's fine ?
---
doc/src/sgml/ref/psql-ref.sgml | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 0595d1c04b..cdd24fad98 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1244,7 +1244,7 @@ testdb=
(see ).
If access-method-patttern
is specified, only operator classes associated with access methods whose
-names match pattern are listed.
+names match the pattern are listed.
If input-type-pattern
is specified, only operator classes associated with input types whose
names match the pattern are listed.
@@ -1267,7 +1267,7 @@ testdb=
