Re: delta relations in AFTER triggers

On 09/15/2014 05:25 PM, Kevin Grittner wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Heikki Linnakangas <hlinnakangas(at)vmware(dot)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