I haven't been following this thread closely, but I looked briefly at
some of the patches posted here:
On 21/01/2019 11:01, Andres Freund wrote:
> The patchset is now pretty granularly split into individual pieces.
Wow, 42 patches, very granular indeed! That's nice for reviewing, but
are you planning to squash them before committing? Seems a bit excessive
for the git history.
Patches 1-4:
* v12-0001-WIP-Introduce-access-table.h-access-relation.h.patch
* v12-0002-Replace-heapam.h-includes-with-relation.h-table..patch
* v12-0003-Replace-uses-of-heap_open-et-al-with-table_open-.patch
* v12-0004-Remove-superfluous-tqual.h-includes.patch
These look good to me. I think it would make sense to squash these
together, and commit now.
Patches 20 and 21:
* v12-0020-WIP-Slotified-triggers.patch
* v12-0021-WIP-Slotify-EPQ.patch
I like this slotification of trigger and EPQ code. It seems like a nice
thing to do, independently of the other patches. You said you wanted to
polish that up to committable state, and commit separately: +1 on that.
Perhaps do that even before patches 1-4.
> --- a/src/include/commands/trigger.h
> +++ b/src/include/commands/trigger.h
> @@ -35,8 +35,8 @@ typedef struct TriggerData
> HeapTuple tg_trigtuple;
> HeapTuple tg_newtuple;
> Trigger *tg_trigger;
> - Buffer tg_trigtuplebuf;
> - Buffer tg_newtuplebuf;
> + TupleTableSlot *tg_trigslot;
> + TupleTableSlot *tg_newslot;
> Tuplestorestate *tg_oldtable;
> Tuplestorestate *tg_newtable;
> } TriggerData;
Do we still need tg_trigtuple and tg_newtuple? Can't we always use the
corresponding slots instead? Is it for backwards-compatibility with
user-defined triggers written in C? (Documentation also needs to be
updated for the changes in this struct)
I didn't look a the rest of the patches yet...
- Heikki
