Re: Code checks for App Devs, using new options for transaction behavior

On Mon, Oct 31, 2022 at 5:03 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Mon, Oct 31, 2022 at 4:23 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Sun, Oct 30, 2022 at 11:32 PM Simon Riggs
> > <simon(dot)riggs(at)enterprisedb(dot)com> wrote:
> > >
> > > On Fri, 28 Oct 2022 at 10:33, Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com> wrote:
> > >
> > > > Thanks for the feedback, I will make all of those corrections in the
> > > > next version.
> > >
> > > New version attached. I've rolled 002-004 into one patch, but can
> > > split again as needed.
> >
> > I like the idea of "parse only" and "nested xact", thanks for working
> > on this. I will look into patches in more detail, especially nested
> > xact. IMHO there is no point in merging "nested xact" and "rollback on
> > commit". They might be changing the same code location but these two
> > are completely different ideas, in fact all these three should be
> > reviewed as three separate threads as you mentioned in the first email
> > in the thread.
>
> What is the behavior if "nested_transactions" value is changed within
> a transaction execution, suppose the value was on and we have created
> a few levels of nested subtransactions and within the same transaction
> I switched it to off or to outer?

1.
@@ -3815,6 +3861,10 @@ PrepareTransactionBlock(const char *gid)
/* Set up to commit the current transaction */
result = EndTransactionBlock(false);

+ /* Don't allow prepare until we are back to an unnested state at level 0 */
+ if (XactNestingLevel > 0)
+ return false;

I am not sure whether it is good to not allow PREPARE or we can just
prepare it and come out of the complete nested transaction. Suppose
we have multiple savepoints and we say prepare then it will just
succeed so why does it have to be different here?

2. case TBLOCK_SUBABORT:
ereport(WARNING,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("there is already a transaction in progress")));
+ if (XactNesting == XACT_NEST_OUTER)
+ XactNestingLevel++;
break;

I did not understand what this change is for, can you tell me the
scenario or a test case to hit this?

Remaining part w.r.t "nested xact" patch looks fine, I haven't tested
it yet though.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com