Re: doc: Clarify Savepoint Behavior

Поиск
Список
Период
Сортировка
От David G. Johnston
Тема Re: doc: Clarify Savepoint Behavior
Дата
Msg-id CAKFQuwaCe-oHVLuXL0e=54EredKUPy8YEHCyuw=x17ycbBK+fw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: doc: Clarify Savepoint Behavior  (Simon Riggs <simon.riggs@enterprisedb.com>)
Ответы Re: doc: Clarify Savepoint Behavior  (Bruce Momjian <bruce@momjian.us>)
Список pgsql-hackers
Thank you for the review.

On Thu, Jun 23, 2022 at 5:35 AM Simon Riggs <simon.riggs@enterprisedb.com> wrote:
On Thu, 9 Jun 2022 at 16:41, David G. Johnston
<david.g.johnston@gmail.com> wrote:

"The name to give to the new savepoint.  The name may already exist,
+      in which case a rollback or release to the same name will use the
+      one that was most recently defined."

instead I propose:

"The name to give to the new savepoint. If the name duplicates a
 previously defined savepoint name then only the latest savepoint with that name
 can be referenced in a later ROLLBACK TO SAVEPOINT."

So leave the "release" behavior implied from the rollback behavior?

On the whole I'm slightly in favor of your proposed wording (mostly due to the better fitting of the ROLLBACK command, though at the omission of RELEASE...) but are you seeing anything beyond personal style as to why you feel one is better than the other?  Is there some existing wording in the docs that I should be conforming to here?


+  <para>
+   If multiple savepoints have the same name, only the one that was most
+   recently defined is released.
+  </para>

instead I propose

"Searches backwards through previously defined savepoints until the
 a savepoint name matches the request. If the savepoint name duplicated earlier
 defined savepoints then those earlier savepoints can only be released if
 multiple ROLLBACK TO SAVEPOINT commands are issued with the same
 name, as shown in the following example."


Upon reflection, adding this after the comments about cursors seems like a poor location, I will probably move it up one paragraph.

I dislike this proposal for its added wordiness that doesn't bring in new material.  The whole idea of "searching backwards until the name is found" is already covered in the description:

"ROLLBACK TO SAVEPOINT implicitly destroys all savepoints that were established after the named savepoint."

Using the phrase "can only be released if" here in the rollback to savepoint command page just seems to be asking for confusion between this and the release savepoint command.

Also, I would just call the savepoint "s" in the example, to declutter it.


If I do use a name that differs from the other two examples on that page I'll probably go with "sp" for added detectability - but deviating from the established convention doesn't seem warranted here.

In all, I'm still content with the patch as-is; though I or the committer should consider moving up the one paragraph in rollback to savepoint.  Otherwise I'll probably post an updated patch sometime this coming week and give another look at the savepoint name description and make that paragraph move.

David J.

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Justin Pryzby
Дата:
Сообщение: Re: Add LZ4 compression in pg_dump
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: Support logical replication of DDLs