Re: Last gasp

On Thu, 2012-04-05 at 15:02 -0400, Robert Haas wrote:
> On Thu, Apr 5, 2012 at 2:46 PM, Hannu Krosing <hannu(at)krosing(dot)net> wrote:
> > To me it looked like the scope of the patch started to suddenly expand
> > exponentially a few days ago from a simple COMMAND TRIGGERS, which would
> > have finally enabled trigger-based or "logical" replication systems to
> > do full replication to something recursive which would attempt to cover
> > all weird combinations of commands triggering other commands for which
> > there is no real use-case in view, except a suggestion "don't do it" :)
> >
> > The latest patch (v18) seemed quite ok for its original intended
> > purpose.
>
> OK, so here we go, rehashing the discussion we already had on thread A
> on thread B. The particular issue you are mentioning there was not
> the reason that patch isn't going to end up in 9.2. If the only thing
> the patch had needed was a little renaming and syntax cleanup, I would
> have done it myself (or Dimitri would have) and I would have committed
> it. That is not the problem, or at least it's not the only problem.
> There are at least two other major issues:
>
> - The patch as posted fires triggers at unpredictable times depending
> on which command you're executing. Some things that are really
> sub-commands fire triggers anyway as if they were toplevel commands;
> others don't; whether or not it happens in a particular case is
> determined by implementation details rather than by any consistent
> principle of operation. In the cases where triggers do fire, they
> don't always fire at the same place in the execution sequence.

For me it would be enough to know if the trigger is fired by top-level
command or not.

In worst case I could probably detect it myself, just give me something
to hang the detection code to .

> - The patch isn't safe if the triggers try to execute DDL on the
> object being modified. It's not exactly clear what misbehavior will
> result in every case, but it is clear that that it hasn't really been
> thought about.

It never seemed important for me, as the only thing I was ever expecting
to do in a command trigger was inserting rows in one unrelated table.

To me DDL-triggered-by-DDL seemed like a very bad idea anyway.

But as you seemed to be envisioning some use-cases for that I did not
object to you working it out.

> Now, if anyone who was actually following the conversation thought
> these things were not problems, they could have written back to the
> relevant thread and said, hey, I don't mind if the trigger firing
> behavior changes every time someone does any refactoring of our
> badly-written DDL code and if the server blows up in random ways when
> someone does something unexpected in the trigger that's OK with me
> too.

I don't mind if the trigger firing behavior changes every time someone
does any refactoring of our badly-written DDL code

Here :)

> Maybe not surprisingly, no one said that. Two people wrote into
> that thread after my latest round of reviewing and both of them
> disagreed with only minor points of my review, and we had a technical
> discussion about those issues. But showing up after the fact and
> acting as if there were no serious issues found during that review is
> either disingenuous or a sign that you didn't really read the thread.

As there are other ways to blow up the backend, i did not object to you
either working out a better solution or leaving it as it is.

I am speaking up now as this is the first time I am told I have to wait
another year for this feature to arrive.

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