[doc] fix a potential grammer mistake
I think in the following sentence, were should be replaced with have,
what do you think?
```
/*
- * We were just issued a SAVEPOINT inside a
transaction block.
+ * We have just issued a SAVEPOINT inside a
transaction block.
* Start a subtransaction. (DefineSavepoint already did
* PushTransaction, so as to have someplace to
put the SUBBEGIN
* state.)
```--
Regards
Junwang Zhao
Attachments:
0001-doc-fix-a-potential-grammer-mistake.patchapplication/octet-stream; name=0001-doc-fix-a-potential-grammer-mistake.patchDownload
From b3e17fe1a504958dba35b19930f1437323f235d7 Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Wed, 3 Aug 2022 16:07:23 +0800
Subject: [PATCH v1] [doc] fix a potential grammer mistake
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
src/backend/access/transam/xact.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index ce1417b8f0..a0a28281dd 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -3140,7 +3140,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were just issued a SAVEPOINT inside a transaction block.
+ * We have just issued a SAVEPOINT inside a transaction block.
* Start a subtransaction. (DefineSavepoint already did
* PushTransaction, so as to have someplace to put the SUBBEGIN
* state.)
@@ -3151,7 +3151,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were issued a RELEASE command, so we end the current
+ * We have issued a RELEASE command, so we end the current
* subtransaction and return to the parent transaction. The parent
* might be ended too, so repeat till we find an INPROGRESS
* transaction or subtransaction.
@@ -3168,7 +3168,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were issued a COMMIT, so we end the current subtransaction
+ * We have issued a COMMIT, so we end the current subtransaction
* hierarchy and perform final commit. We do this by rolling up
* any subtransactions into their parent, which leads to O(N^2)
* operations with respect to resource owners - this isn't that
--
2.33.0
On 3 Aug 2022, at 10:10, Junwang Zhao <zhjwpku@gmail.com> wrote:
I think in the following sentence, were should be replaced with have,
what do you think?``` /* - * We were just issued a SAVEPOINT inside a transaction block. + * We have just issued a SAVEPOINT inside a transaction block. * Start a subtransaction. (DefineSavepoint already did * PushTransaction, so as to have someplace to put the SUBBEGIN * state.) ```
I'm not so sure. If I read this right the intent of the sentence is to convey
that the user has issued a SAVEPOINT to the backend, not the backend itself. I
think the current wording is the correct one.
--
Daniel Gustafsson https://vmware.com/
Op 03-08-2022 om 10:10 schreef Junwang Zhao:
I think in the following sentence, were should be replaced with have,
what do you think?``` /* - * We were just issued a SAVEPOINT inside a transaction block. + * We have just issued a SAVEPOINT inside a transaction block. * Start a subtransaction. (DefineSavepoint already did * PushTransaction, so as to have someplace to put the SUBBEGIN * state.) ```
I don't think these "were"s are wrong but arguably changing them to
"have" helps non-native speakers (like myself), as it doesn't change the
meaning significantly as far as I can see.
'we were issued' does reflect the perspective of the receiving code a
bit better.
Erik
On Wed, Aug 3, 2022 at 4:23 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 3 Aug 2022, at 10:10, Junwang Zhao <zhjwpku@gmail.com> wrote:
I think in the following sentence, were should be replaced with have,
what do you think?``` /* - * We were just issued a SAVEPOINT inside a transaction block. + * We have just issued a SAVEPOINT inside a transaction block. * Start a subtransaction. (DefineSavepoint already did * PushTransaction, so as to have someplace to put the SUBBEGIN * state.) ```I'm not so sure. If I read this right the intent of the sentence is to convey
that the user has issued a SAVEPOINT to the backend, not the backend itself. I
think the current wording is the correct one.
Got it, using `were` here means the backend is the receiver of the
action, not the sender.
That makes sense, thanks a lot.
--
Daniel Gustafsson https://vmware.com/
--
Regards
Junwang Zhao
yeah, not a grammar mistake at all, "were" should be used here, thanks
for pointing that out ;)
On Wed, Aug 3, 2022 at 4:27 PM Erikjan Rijkers <er@xs4all.nl> wrote:
Op 03-08-2022 om 10:10 schreef Junwang Zhao:
I think in the following sentence, were should be replaced with have,
what do you think?``` /* - * We were just issued a SAVEPOINT inside a transaction block. + * We have just issued a SAVEPOINT inside a transaction block. * Start a subtransaction. (DefineSavepoint already did * PushTransaction, so as to have someplace to put the SUBBEGIN * state.) ```I don't think these "were"s are wrong but arguably changing them to
"have" helps non-native speakers (like myself), as it doesn't change the
meaning significantly as far as I can see.'we were issued' does reflect the perspective of the receiving code a
bit better.Erik
--
Regards
Junwang Zhao
Erikjan Rijkers <er@xs4all.nl> writes:
I don't think these "were"s are wrong but arguably changing them to
"have" helps non-native speakers (like myself), as it doesn't change the
meaning significantly as far as I can see.
I think it does --- it changes the meaning from passive to active.
I don't necessarily object to rewriting these sentences more broadly,
but I don't think "have issued" is the correct phrasing.
Possibly "The user issued ..." would work.
regards, tom lane
Attachment is a corrected version based on Tom's suggestion.
Thanks.
On Wed, Aug 3, 2022 at 9:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Erikjan Rijkers <er@xs4all.nl> writes:
I don't think these "were"s are wrong but arguably changing them to
"have" helps non-native speakers (like myself), as it doesn't change the
meaning significantly as far as I can see.I think it does --- it changes the meaning from passive to active.
I don't necessarily object to rewriting these sentences more broadly,
but I don't think "have issued" is the correct phrasing.Possibly "The user issued ..." would work.
regards, tom lane
--
Regards
Junwang Zhao
Attachments:
0001-doc-rewrite-some-comments-to-make-them-more-precise.patchapplication/octet-stream; name=0001-doc-rewrite-some-comments-to-make-them-more-precise.patchDownload
From 0fcad9e23877d6d8a14904c75fd8f738fc35f9f1 Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Wed, 3 Aug 2022 16:07:23 +0800
Subject: [PATCH v1] [doc] rewrite some comments to make them more precise
The `We` in the sentences is not clear to be understood,
change to `The user` to make it clear.
Author: Junwang Zhao, Tom Lane
Reviewed-by: Daniel Gustafsson, Erikjan Rijkers
---
src/backend/access/transam/xact.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index ce1417b8f0..37d23db62c 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -3140,7 +3140,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were just issued a SAVEPOINT inside a transaction block.
+ * The user just issued a SAVEPOINT inside a transaction block.
* Start a subtransaction. (DefineSavepoint already did
* PushTransaction, so as to have someplace to put the SUBBEGIN
* state.)
@@ -3151,7 +3151,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were issued a RELEASE command, so we end the current
+ * The user issued a RELEASE command, so we end the current
* subtransaction and return to the parent transaction. The parent
* might be ended too, so repeat till we find an INPROGRESS
* transaction or subtransaction.
@@ -3168,7 +3168,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were issued a COMMIT, so we end the current subtransaction
+ * The user issued a COMMIT, so we end the current subtransaction
* hierarchy and perform final commit. We do this by rolling up
* any subtransactions into their parent, which leads to O(N^2)
* operations with respect to resource owners - this isn't that
--
2.33.0
On Wed, Aug 3, 2022 at 11:15 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
Attachment is a corrected version based on Tom's suggestion.
Thanks.
On Wed, Aug 3, 2022 at 9:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Erikjan Rijkers <er@xs4all.nl> writes:
I don't think these "were"s are wrong but arguably changing them to
"have" helps non-native speakers (like myself), as it doesn't change the
meaning significantly as far as I can see.I think it does --- it changes the meaning from passive to active.
I don't necessarily object to rewriting these sentences more broadly,
but I don't think "have issued" is the correct phrasing.Possibly "The user issued ..." would work.
Is there a reason that the first case says "just" issued vs the other
two cases? It seems to me that it should be removed.
Robert Treat
https://xzilla.net
On Thu, Aug 4, 2022 at 12:42 AM Robert Treat <rob@xzilla.net> wrote:
On Wed, Aug 3, 2022 at 11:15 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
Attachment is a corrected version based on Tom's suggestion.
Thanks.
On Wed, Aug 3, 2022 at 9:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Erikjan Rijkers <er@xs4all.nl> writes:
I don't think these "were"s are wrong but arguably changing them to
"have" helps non-native speakers (like myself), as it doesn't change the
meaning significantly as far as I can see.I think it does --- it changes the meaning from passive to active.
I don't necessarily object to rewriting these sentences more broadly,
but I don't think "have issued" is the correct phrasing.Possibly "The user issued ..." would work.
Is there a reason that the first case says "just" issued vs the other
two cases? It seems to me that it should be removed.
Attachment is a patch with the "just" removed.
Thanks
Robert Treat
https://xzilla.net
--
Regards
Junwang Zhao
Attachments:
0001-doc-rewrite-some-comments-to-make-them-more-precise.patchapplication/octet-stream; name=0001-doc-rewrite-some-comments-to-make-them-more-precise.patchDownload
From f6220b33b286cfd2e6d52017347ecf82d03c5c20 Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Wed, 3 Aug 2022 16:07:23 +0800
Subject: [PATCH v1] [doc] rewrite some comments to make them more precise
The `We` in the sentences is not clear to be understood,
change to `The user` to make it clear.
Author: Junwang Zhao, Tom Lane
Reviewed-by: Daniel Gustafsson, Erikjan Rijkers, Robert Treat
---
src/backend/access/transam/xact.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index ce1417b8f0..50f092d7eb 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -3140,7 +3140,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were just issued a SAVEPOINT inside a transaction block.
+ * The user issued a SAVEPOINT inside a transaction block.
* Start a subtransaction. (DefineSavepoint already did
* PushTransaction, so as to have someplace to put the SUBBEGIN
* state.)
@@ -3151,7 +3151,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were issued a RELEASE command, so we end the current
+ * The user issued a RELEASE command, so we end the current
* subtransaction and return to the parent transaction. The parent
* might be ended too, so repeat till we find an INPROGRESS
* transaction or subtransaction.
@@ -3168,7 +3168,7 @@ CommitTransactionCommand(void)
break;
/*
- * We were issued a COMMIT, so we end the current subtransaction
+ * The user issued a COMMIT, so we end the current subtransaction
* hierarchy and perform final commit. We do this by rolling up
* any subtransactions into their parent, which leads to O(N^2)
* operations with respect to resource owners - this isn't that
--
2.33.0
On 4 Aug 2022, at 00:44, Junwang Zhao <zhjwpku@gmail.com> wrote:
Attachment is a patch with the "just" removed.
I think this is a change for better, so I've pushed it. Thanks for the
contribution!
--
Daniel Gustafsson https://vmware.com/
On Thu, Aug 4, 2022 at 10:32 AM Daniel Gustafsson <daniel@yesql.se> wrote:
On 4 Aug 2022, at 00:44, Junwang Zhao <zhjwpku@gmail.com> wrote:
Attachment is a patch with the "just" removed.
I think this is a change for better, so I've pushed it. Thanks for the
contribution!
Thanks!
Robert Treat
https://xzilla.net