Re: Event Triggers: adding information

On Fri, Jan 18, 2013 at 10:47 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> No, there's one also in heap_create_with_catalog. Took me a minute to
>> find it, as it does not use InvokeObjectAccessHook. The idea is that
>> OAT_POST_CREATE fires once per object creation, regardless of the
>> object type - table, column, whatever.
>
> Ah. Chose the wrong term to grep for. I am tempted to suggest adding a
> comment explaining why we InvokeObjectAccessHook isn't used just for
> enhanced grepability.

I cannot parse that sentence, but I agree that the ungreppability of
it is suboptimal. I resorted to git log -SInvokeObjectAccessHook -p
to find it.

>> I have been involved in PostgreSQL development for about 4.5 years
>> now. This is less time than many people here, but it's still long
>> enough to say a whole lot of people ask for some variant of this idea,
>> and yet I have yet to see anybody produce a complete, working version
>> of this functionality and maintain it outside of the PostgreSQL tree
>> for one release cycle (did I miss something?).
>
> I don't really think thats a fair argument.
>
> For one, I didn't really ask for a libpgdump - I said that I don't see
> any way to generate re-executable SQL without it if we don't get the
> parse-tree but only the oid of the created object. Not to speak of the
> complexities of supporting ALTER that way (which is, as you note,
> basically not the way to do this).

Oh, OK. I see. Well, I've got no problem making the parse tree
available via InvokeObjectAccessHook. Isn't that just a matter of
adding a new hook type and a new call site?

> Also, even though I don't think its the right way *for this*, I think
> pg_dump and pgadmin pretty much prove that it's possible? The latter
> even is out-of-core and has existed for multiple years.

Fair point.

> Its also not really fair to compare out-of-tree effort of maintaining
> such a library to in-core support. pg_dump *needs* to be maintained, so
> the additional maintenance overhead once the initial development is done
> shouldn't really be higher than now. Lower if anything, due to getting
> rid of a good bit of ugly code ;). There's no such effect out of core.

This I don't follow. Nothing we might add to core to reverse-parse
either the catalogs or the parse tree is going to make pg_dump go
away.

> If youre also considering parsetree->SQL transformation under the
> libpgdump umbrella its even less fair. The backend already has a *lot*
> of infrastructure to regenerate sql from trees, even if its mostly
> centered arround around DQL. A noticeable amount of that code is
> unavailable to extensions (i.e. many are static funcs).
> Imo its completely unreasonable to expose that code to the outside and
> expect it to have a stable interface. We *will* need to rewrite parts of
> that regularly.
> For those reasons I think the only reasonable way to create textual DDL
> is in the backend trying to allow outside code to do that will impose
> far greater limitations.

I'm having trouble following this. Can you restate? I wasn't sure
what you meant by libpqdump. I assumed you were speaking of a
parsetree->DDL or catalog->DDL facility.

> Well, it is one of the major use-cases (or even *the* major use case)
> for event triggers that I heard of. So it seems a valid point in a
> dicussion on how to design the whole mechanism, doesn't it?

Yes, but is a separate problem from how we give control to the event
trigger (or object access hook).

> Some version of the event trigger patch contained partial support for
> regenerating the DDL so it seems like a justified point there. Your
> complained that suggestions about reusing object access hooks were
> ignored, so mentioning that they currently don't provide *enough* (they
> *do* provide a part, but it doesn't seem to be the most critical one)
> also seems justifyable.

Sure, but we could if we wanted decide to commit the partial support
for regenerating the DDL and tell people to use it via object access
hooks. Of course, that would thin out even further the number of
people who would actually be using that code, which I fear will
already be too small to achieve bug-free operation in a reasonable
time. If we add some hooks now and someone maintains the DDL
reverse-parsing code as an out-of-core replication solution for a few
releases, and that doesn't turn out to be hideous, I'd be a lot more
sanguine about incorporating it at that point. I basically don't
think that there's any way we're going to commit something along those
lines now and not have it turn out to be a far bigger maintenance
headache than anyone wants. What that tends to end up meaning in
practice is that Tom gets suckered into fixing it because nobody else
can take the time, and that's neither fair nor good for the project.

> NP, its good to keep the technical stuff here anyway... Stupid
> technology. In which business are we in again?

I don't know, maybe we can figure it out based on the objects in our
immediate vicinity. I see twelve cans of paint, most of them opened,
and something that looks sort of like a badly-made shed. Does that
help at all? :-)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company