Re: delta relations in AFTER triggers
Thomas Munro <thomas.munro@enterprisedb.com>
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Kevin Grittner <kgrittn@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Haribabu Kommi <kommi.haribabu@gmail.com>,
Jim Nasby <Jim.Nasby@bluetreble.com>, Craig Ringer <craig@2ndquadrant.com>,
Michael Paquier <michael.paquier@gmail.com>, David Fetter <david@fetter.org>, Kevin Grittner <kgrittn@ymail.com>, Amit Khandekar <amit.khandekar@enterprisedb.com>,
Alvaro Herrera <alvherre@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-04-03T03:50:37Z
Lists: pgsql-hackers
Attachments
- spi-portal-open-with-query-env.patch (application/octet-stream) patch
- transition-plpython-v1.patch (application/octet-stream) patch v1
- transition-plperl-v1.patch (application/octet-stream) patch v1
- transition-pltcl-v1.patch (application/octet-stream) patch v1
Happy to see this committed! And thanks for the co-author credit,
which is a generous exaggeration.
I was still a bit confused about this and poked at it a bit:
On Wed, Mar 8, 2017 at 1:28 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
>> /*
>> + * Capture the NEW and OLD transition TABLE tuplestores (if specified for
>> + * this trigger).
>> + */
>> + if (trigdata->tg_newtable || trigdata->tg_oldtable)
>> + {
>> + estate.queryEnv = create_queryEnv();
>> + if (trigdata->tg_newtable)
>> + {
>> + Enr enr = palloc(sizeof(EnrData));
>> +
>> + enr->md.name = trigdata->tg_trigger->tgnewtable;
>> + enr->md.tupdesc = trigdata->tg_relation->rd_att;
>> + enr->md.enrtuples = tuplestore_tuple_count(trigdata->tg_newtable);
>> + enr->reldata = trigdata->tg_newtable;
>> + register_enr(estate.queryEnv, enr);
>> + SPI_register_relation(enr);
>> + }
>>
>> Why do we we have to call register_enr and also SPI_register_relation here?
>
> Essentially, because plpgsql does some things through SPI and some
> things not. Both cases are covered.
We're maintaining two different QueryEnvironment objects here, one
inside the SPI module and another in plpgsql_EState. I think that's
done only so that we have one to inject into the portal in
exec_dynquery_with_params, so that EXECUTE 'SELECT * FROM
<transition_table>' can work.
That raises the question: shouldn't SPI_cursor_open just do that
itself using the SPI connection's current QueryEnvironment? That
would make SPI_cursor_open consistent with SPI_execute_plan, and also
benefit handlers for other PLs that would otherwise have to do similar
double-bookkeeping. See attached patch showing what I mean.
Please also find attached a rebased patch to add pl/python support,
and new equivalent patches for pl/perl and pl/tcl. I am planning to
add these to PG11 CF1, unless you think we should be more aggressive
given the extra time?
--
Thomas Munro
http://www.enterprisedb.com
Commits
-
Don't permit transition tables with TRUNCATE triggers.
- 29fd3d9da0ff 10.0 landed
-
Pass EXEC_FLAG_REWIND when initializing a tuplestore scan.
- 304007d9f1f6 10.0 landed
-
Faster expression evaluation and targetlist projection.
- b8d7f053c5c2 10.0 cited
-
Remove obsoleted code relating to targetlist SRF evaluation.
- ea15e18677fc 10.0 cited
-
Implement syntax for transition tables in AFTER triggers.
- 8c48375e5f43 10.0 cited