Re: Add CREATE support to event triggers - Mailing list pgsql-hackers

On 2014-11-08 12:30:29 -0500, Robert Haas wrote:
> On Sat, Nov 8, 2014 at 12:20 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> >> Putting half of it into core wouldn't fix that, it would just put a
> >> lot more maintenance burden on core developers.
> >
> > Imo stuff that can't be done sanely outside core needs to be put into
> > core if it's actually desired by many users. And working DDL replication
> > for logical replication solutions surely is.
> 
> I don't buy it.  This patch series is *all about* transferring the
> maintenance burden of this feature from the BDR developers to the core
> project.

What? Not at all. It's *much* less work to do these kind of things out
of core. As you probably have experienced more than once. If it were
possible to do this entirely in a extension I'm pretty sure nobody would
have bothered.

> There's nothing to keep you from exposing the parse trees to
> C functions that can live in an extension, and you can do all of this
> deparsing there.

Not really. There's some core functions that need to be touched. Like
most of the stuff in patches 1,2,3,5,16 does.

We could just integrate those parts, and be done with it. But would that
actually be a good thing for the community? Then slony needs to do it
and potentially others as well? Then auditing can't use it? Then
potential schema tracking solutions can't use it?

> Nobody will stop you, and when it breaks (not if)
> you can fix it in your code. The overhead of deparsing new types of
> parse nodes can be born by you.  The only benefit of pushing it into
> core is that some other logical replication solution could also take
> advantage of that, but I know of nobody with any plans to do such a
> thing.

There've been people for a long while asking about triggers on catalogs
for that purpose. IIRC Jan was one of them.

> On the flip side, the *cost* of pushing it into core is that
> it now becomes the PostgreSQL's community problem to update the
> deparsing code every time the grammar changes.  That cost-benefit
> trade-off does not look favorable to me, especially given the absence
> of any kind of robust testing framework.

I agree that there should be testing in this.

But I think you're quite overstating the effort of maintaining
this. Looking at gram.y between the stamping of 9.3 devel and 9.4 devel
for commits that'd require DDL deparsing changes:
* ALTER TABLE .. ALTER CONSTRAINT: About 7 lines for the deparse code, out of a ~350 line patch
* REFRESH MATERIALIZED VIEW ... CONCURRENTLY: About 1 line for deparse, out of ~700
* WITH CHECK OPTION for views: About 3 lines for deparse, out of ~1300
* REPLICA IDENTITY: About 24 lines for deparse, out of ~950
* ALTER TABLESPACE MOVE: About 20 lines for deparse, out of ~340. Although that patch was essentially scrapped
afterwards.And rewritten differently.
 
* CREATE TABLESPACE ... WITH ...:  Not supported by event triggers right now as it's a global object.

That's really not a whole lot.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services