Обсуждение: Re: Document How Commit Handles Aborted Transactions
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation: tested, failed Summary: --------- The patch adds documentation to clarify how PostgreSQL handles aborted transactions during the commit process. The changesare clear and improve the existing documentation. Testing: -------- 1. Manually applied the patch to the latest master branch (commit 4cffc93). 2. Fixed SGML structure issues in `advanced.sgml` and `commit.sgml` by wrapping `<varlistentry>` in `<variablelist>`. 3. Rebuilt the documentation using `make html`. 4. Verified the new sections are present and correctly formatted in the generated HTML. Feedback: --------- - The patch was manually applied due to conflicts in `advanced.sgml` and `commit.sgml`. - Fixed invalid SGML structure by wrapping `<varlistentry>` in `<variablelist>`. - The documentation is accurate and follows PostgreSQL’s style guidelines. - No additional issues were found. Recommendation: --------------- Ready for committer. No objections. The new status of this patch is: Ready for Committer
Ahmed,
Thank you for the review.
I'm a bit confused by the reports of apply and compile errors. I didn't touch anything involving "varlist" and see no errors then or now in my Meson ninja build. Nor does the CI report any.
On Fri, Feb 14, 2025 at 2:01 PM Ahmed Ashour <a8087027@gmail.com> wrote:
Feedback:
---------
- The patch was manually applied due to conflicts in `advanced.sgml` and `commit.sgml`.
- Fixed invalid SGML structure by wrapping `<varlistentry>` in `<variablelist>`.
If those errors were/are real this wouldn't be ready to commit. But as they seem to be a local environment issue on your end, and you agree with the content, I'll keep it ready to commit.
David J.
On Thu, 29 May 2025 at 04:32, David G. Johnston <david.g.johnston@gmail.com> wrote: > > Recent discussion led me to realize we are also contrary to the SQL Standard here. v3 updates the Commit reference pageto reflect this fact. > > Leaving ready-to-commit. > > David J. > Hi! I reviewed your changes and I agree with them. The only aspect that drew my attention is in the following sentence: >+ but instead goes into an aborted state. While in this state all commands except > + <xref linkend="sql-commit"/> and <xref linkend="sql-rollback"/> are ignored We can also do ABORT; Is this worth noting? -- Best regards, Kirill Reshke
On Friday, August 22, 2025, Kirill Reshke <reshkekirill@gmail.com> wrote:
We can also do ABORT;
Listing commands described as being “present for historical reasons” in the documentation seems unnecessary. We don’t list abort in the “See Also” section either.
David J.
On Fri, 22 Aug 2025 at 18:31, David G. Johnston <david.g.johnston@gmail.com> wrote: > > On Friday, August 22, 2025, Kirill Reshke <reshkekirill@gmail.com> wrote: >> >> >> We can also do ABORT; >> > > Listing commands described as being “present for historical reasons” in the documentation seems unnecessary. We don’tlist abort in the “See Also” section either. > > David J. > I bumped into specific behaviour of COMMIT in an already-broken transaction yet again recently. So, I moved CF entry to the next CF, hope this will get eventually merged. LGTM -- Best regards, Kirill Reshke