Re: delta relations in AFTER triggers

On 09/15/2014 05:25 PM, Kevin Grittner wrote:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
>>
>>> On 08/30/2014 12:15 AM, Kevin Grittner wrote:
>>>> If we were to go with the hooks as you propose, we would still need
>>>> to take the information from TriggerData and put it somewhere else
>>>> for the hook to reference.
>>
>>> Sure.
>>
>> FWIW, I agree with Heikki on this point.  It makes a lot more sense for
>> the parser to provide hooks comparable to the existing hooks for resolving
>> column refs, and it's not apparent that loading such functionality into
>> SPI is sane at all.
>>
>> OTOH, I agree with Kevin that the things we're talking about are
>> lightweight relations not variables.
>
> Try as I might, I was unable to find any sensible way to use hooks.
> If the issue was only the parsing, the route was fairly obvious,
> but the execution side needs to access the correct tuplestore(s)
> for each run, too -- so the sort of information provided by
> relcache needed to be passed in to based on the context within the
> process (i.e., if you have nested triggers firing, each needs to
> use a different tuplestore with the same name in the same
> function, even though it's using the same plan).  On both sides it
> seemed easier to pass things through the same sort of techniques as
> "normal" parameters; I couldn't find a way to use hooks that didn't
> just make things uglier.

Hmph. You didn't actually use the same sort of techniques we use for 
normal parameters. You invented a new TsrCache registry, which marries 
the metadata for planning with the tuplestore itself. That's quite 
different from the way we deal with parameters (TsrCache is a misnomer, 
BTW; it's not a cache, but the primary source of information for the 
planner). And instead of passing parameters to the SPI calls 
individually, you invented SPI_register_tuplestore which affects all 
subsequent SPI calls.

To recap, this is how normal parameters work:

In the parse stage, you pass a ParserSetupHook function pointer to the 
parser. The parser calls the callback, which sets up more hooks in the 
ParseState struct: a column-ref hook and/or a param ref hook. The parser 
then proceeds to parse the query, and whenever it sees a reference to a 
column that it doesn't recognize, or a $n style parameter marker, it 
calls the column-ref or param-ref hook.

The column- or param-ref hook can return a Param node, indicating that 
the parameter's value will be supplied later, at execution time. The 
Param node contains a numeric ID for the parameter, and the type OID and 
other information needed to complete the parsing.

At execution time, you pass a ParamListInfo struct to the executor. It 
contains values for all the parameters. Alternatively, the values can be 
supplied lazily, by providing a param-fetch hook in the ParamListInfo 
struct. Whenever the executor needs the value of a parameter, and the 
ParamListInfo struct doesn't contain it, it calls the paramFetch hook 
which should fill it in ParamListInfo.

Now, how do we make the tuplestores work similarly? Here's what I think 
we should do:

Add a new p_tableref_hook function pointer, similar to p_paramref_hook. 
Whenever the parser sees a RangeVar that it doesn't recognize (or 
actually, I think it should call it *before* resolving regular tables, 
but let's ignore that for now), it calls the p_tableref_hook. It can 
return a new RelationParam node (similar to regular Param), which 
contains a numeric ID for the table/tuplestore, as well as its tuple 
descriptor.

For the execution phase, add a new array of Tuplestorestates to 
ParamListInfo. Similar to the existing array of ParamExternalDatas.

The next question is how to pass the new hooks and tuplestores through 
the SPI interface. For prepared plans, the current SPI_prepare_params + 
SPI_execute_plan_with_paramlist functions work fine. However, there 
doesn't seem to be any way to do one-shot queries with a ParserSetupHook 
and ParamListInfo. That seems like an oversight, and would be nice to 
address that anyway.

PS. the copy/read/write functions for TuplestoreRelation in the patch 
are broken; TuplestoreRelation is not a subclass of Plan. (But if you go 
with the approach I'm advocating for, TuplestoreRelation in its current 
form will be gone)

- Heikki