( Added Joe and Robert for 0011 )
On Mon, 11 Apr 2022 at 14:03, Justin Pryzby <pry...@telsasoft.com> wrote:
> In docs and comments. Mostly for v15.
0001:
Should this not use <productname>PostgreSQL</productname>? (new to master)
0002:
The patch looks good. (new to v12)
0003:
The patch looks good. (new to master)
0004:
The patch looks good. (new to master)
0005:
I'm not entirely certain this is an improvement. Your commit message
I'd say is not true going by git grep "compression algorithm". There
are 3 matches in the docs and take [1], for example. I'd say in that
one it's better to use "algorithm". In that case, "method" could be
talking about client or server.
That makes me wonder if the change you're proposing is an improvement or not.
0006:
The patch looks good. (new to master)
0007:
- When the <option>--max-tries</option> option is used, the
transaction with
- serialization or deadlock error cannot be retried if the total time of
+ When the <option>--max-tries</option> option is used, a
transaction with
+ serialization or deadlock error will not be retried if the
total time of
Shouldn't this be slightly clearer and say "a transaction which fails
due to a serialization anomaly or a deadlock"?
- database server / the syntax error in the meta command / thread
+ database server / syntax error in the meta command / thread
Should we not separate these items out with commas?
- the client is aborted. Otherwise, if an SQL fails with serialization or
+ the client is aborted. Otherwise, if an SQL command fails with
serialization or
deadlock errors, the client is not aborted. In such cases, the current
I'd say "if an SQL command fails due to a serialization anomaly or due
to deadlocking".
(new to master)
0008:
The patch looks good. (new to master)
0009:
The patch looks good. (new to master)
0010:
I don't understand this change.
0011:
I can't quite parse the original. I might not have enough context
here. Robert, Joe? (new to master)
0012:
This seems to contain some documentation fixes too. The patch claims
it's just for comments.
- * pages that are outwith that range.
+ * pages that are outside that range.
I personally don't see the issue here, but I'm Scottish. I think the
best transaction is just "outside of" rather than replacing with just
"outside".
All the other changes look fine.
0013:
I think we should fix all these, regardless of how old the mistake is.
0014:
- * shouldn't PANIC just because we can't guarantee the the backup has been
+ * shouldn't PANIC just because we can't guarantee the backup has been
"that the"
0015:
Patch looks fine.
0016:
0017:
I'm not really sure if we should fix these or not. From having a
quick look at some of them it seems we'd be adding churn to some
pretty old code. Happy to hear what others think.
0018:
The patch looks good.
0019:
-1. pgindent will fix these.
I will start pushing the less controversial of these, after a bit of squashing.
David
[1] https://www.postgresql.org/docs/devel/app-pgbasebackup.html
