Subject: Re: Last gasp
From: Hannu Krosing <hannu@krosing.net>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Simon Riggs <simon@2ndquadrant.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
In-Reply-To: 
 <CA+Tgmoa75QujxzCZC+dj=0+JayGPhJD2kQMDBmbGmmq_HPgHxw@mail.gmail.com>
References: 
 <CA+U5nML0w7FAM8Wi6W1ObSa160ipN5shg3Z0y91ew-Um8iXxCg@mail.gmail.com>
	<CA+TgmoazMq6FF2otE_5HYJr3nwwEy9_0TmGYu_G3AsY8ckQcjg@mail.gmail.com>
	<CA+U5nMJLg6aDi-0LOZXx6HMnEKE6-GY7pbCbjTXkQvfSwEb+qw@mail.gmail.com>
	<CA+Tgmoaf=b+QNcR+CZE6TfK-DN=BNV2xDikVZwdreuiC+PnrrQ@mail.gmail.com>
	<1333651599.31440.47.camel@hvost>
	<CA+Tgmoa75QujxzCZC+dj=0+JayGPhJD2kQMDBmbGmmq_HPgHxw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Date: Thu, 05 Apr 2012 21:20:59 +0200
Message-ID: <1333653659.31440.68.camel@hvost>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit

On Thu, 2012-04-05 at 15:02 -0400, Robert Haas wrote:
> On Thu, Apr 5, 2012 at 2:46 PM, Hannu Krosing <hannu@krosing.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
>