On Wed, Apr 17, 2024 at 2:37 PM Alexander Korotkov <aekorot...@gmail.com> wrote: > On Tue, Apr 16, 2024 at 7:42 PM Alexander Korotkov <aekorot...@gmail.com> > wrote: > > On Tue, Apr 16, 2024 at 6:35 PM Andres Freund <and...@anarazel.de> wrote: > > > On 2024-04-16 15:45:42 +0300, Alexander Korotkov wrote: > > > > On Tue, Apr 16, 2024 at 1:48 AM Andres Freund <and...@anarazel.de> > > > > wrote: > > > > > On 2024-03-06 14:17:23 +0200, Alexander Korotkov wrote: > > > > > > 0001 Turn tail recursion into iteration in > > > > > > CommitTransactionCommand() > > > > > > I did minor revision of comments and code blocks order to improve > > > > > > the > > > > > > readability. > > > > > > > > > > After sending > > > > > https://www.postgresql.org/message-id/20240414223305.m3i5eju6zylabvln%40awork3.anarazel.de > > > > > I looked some more at important areas where changes didn't have code > > > > > coverage. One thing I noticed was that the "non-internal" part of > > > > > AbortCurrentTransaction() is uncovered: > > > > > https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/access/transam/xact.c.gcov.html#L3403 > > > > > > > > > > Which made me try to understand fefd9a3fed2. I'm a bit confused > > > > > about why > > > > > some parts are handled in > > > > > CommitCurrentTransaction()/AbortCurrentTransaction() > > > > > and others are in the *Internal functions. > > > > > > > > > > I understand that fefd9a3fed2 needed to remove the recursion in > > > > > CommitTransactionCommand()/AbortCurrentTransaction(). But I don't > > > > > understand > > > > > why that means having some code in in the non-internal and some in the > > > > > internal functions? Wouldn't it be easier to just have all the state > > > > > handling > > > > > code in the Internal() function and just break after the > > > > > CleanupSubTransaction() calls? > > > > > > > > I'm not sure I correctly get what you mean. Do you think the attached > > > > patch matches the direction you're pointing? The patch itself is not > > > > final, it requires cleanup and comments revision, just to check the > > > > direction. > > > > > > Something like that, yea. The split does seem less confusing that way to > > > me, > > > but also not 100% certain. > > > > Thank you for your feedback. I'm going to go ahead and polish this patch. > > I've invested more time into polishing this. I'm intended to push > this. Could you, please, take a look before?
Just after sending this I spotted a typo s/untill/until/. The updated version is attached. ------ Regards, Alexander Korotkov
v3-0001-Refactoring-for-CommitTransactionCommand-AbortCur.patch
Description: Binary data
