Re: Pluggable Storage - Andres's take

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Andres Freund <andres(at)anarazel(dot)de>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Asim R P <apraveen(at)pivotal(dot)io>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, aagrawal(at)pivotal(dot)io, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Subject: Re: Pluggable Storage - Andres's take
Date: 2019-02-27 10:00:12
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2019-02-27 10:02:18 Re: Set fallback_application_name for a walreceiver to cluster_name
Previous Message Peter Eisentraut 2019-02-27 09:59:00 Re: Why don't we have a small reserved OID range for patch revisions?