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
