Re: Explicit subtransactions for PL/Tcl

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Victor Wagner <vitus(at)wagner(dot)pp(dot)ru>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Explicit subtransactions for PL/Tcl
Date: 2017-03-11 20:02:12
Message-ID: 28749.1489262532@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> I'll mark this patch as ready for commiter

I've pushed this after mostly-cosmetic cleanup. One thing I changed
that's not cosmetic is to put

MemoryContextSwitchTo(oldcontext);

after the BeginInternalSubTransaction call. I see there was some
discussion upthread about what to do there, but I think this is the
correct answer because it corresponds to what pltcl_subtrans_begin
does. That means that the execution environment of the called Tcl
fragment will be the same as, eg, the loop_body in spi_exec. I do not
think we want it to be randomly different from those pre-existing cases.

Now, I believe that the MemoryContextSwitchTo in pltcl_subtrans_begin
was probably just cargo-culted in there from similar code in plpgsql.
Since pltcl doesn't rely on CurrentMemoryContext to anywhere near the
same degree that plpgsql does, it's possible that we could drop the
MemoryContextSwitchTo in both places, and just let the called code
fragments run in the subtransaction's memory context. But I'm not
convinced of that, and in any case it ought to be undertaken as a
separate patch.

Some other comments:

* Whitespace still wasn't very much per project conventions.
I fixed that by running it through pgindent.

* The golden rule for code style and placement is "make your patch
look like it was always there". Dropping new code at the tail end
of the file is seldom a good avenue to that. I stuck it after
pltcl_SPI_lastoid, on the grounds that it should be with the other
Tcl command functions and should respect the (mostly) alphabetical
order of those functions. Likewise I adopted a header comment
format in keeping with the existing nearby functions.

* I whacked the SGML docs around pretty thoroughly. That addition
wasn't respecting the style of the surrounding text either as to
markup or indentation, and it had some other issues like syntax
errors in the example functions.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-03-11 20:06:47 Re: Need a builtin way to run all tests faster manner
Previous Message Andres Freund 2017-03-11 19:48:31 Re: Need a builtin way to run all tests faster manner