Re: Incorrect processing of CREATE TRANSFORM with DDL deparding

* Michael Paquier (michael(dot)paquier(at)gmail(dot)com) wrote:
> On Tue, May 26, 2015 at 12:52 AM, Alvaro Herrera wrote:
> > Michael Paquier wrote:
> >> In ProcessUtilitySlow()@utility.c, for a node T_CreateTransformStmt,
> >> process does not return ObjectAddress. This makes process inconsistent
> >> with the other commands and the ObjectAddress passed to
> >> EventTriggerCollectSimpleCommand is not initialized.
> >> Coverity has pointed out the error, I just some legwork to sort out a fix.
> >
> > Yeah, I had noticed this and was pretty annoyed because we ended up in
> > precisely the situation we didn't want to be: new code is added to
> > ProcessUtility that is not handled by the deparse framework. (I
> > don't know whether TRANSFORM went in first or deparse, but it doesn't
> > really matter.)
> >
> > The fix as you say is pretty trivial, but I would like to use this is a
> > test case to ensure that we will catch all these mistakes in the future
> > too not only this particular one.
>
> I guess that you could add an assertion at the bottom of
> ProcessUtilitySlow() as well to check code paths where ObjectAddress
> is not initialized correctly.

I seem to recall that being in one of the variations of this patch and I
tend to agree that it makes sense...

That said, I'm really not all that happy with the split between
ProcessUtility() and ProcessUtilitySlow(). I've not said anything since
I haven't got any great solutions to the issue, but it really is pretty
grotty. I realize it might take a few extra cycles, but my thinking is
along the lines of having an array or similar which we scan that
indicates what is supported by deparse/event triggers, what isn't, etc,
and then we operate based on that.. Perhaps an array which is indexed
based on the NodeTag enum? I realize that'd be a stupidly large array,
of, I dunno, 8k or more, but it'd surely make ProcessUtility a heck of a
lot shorter/simpler..

Considering the line count between the two is over 1000, perhaps it'd be
a net savings in size, if not speed.

Just a few off-the-cuff thoughts.

Thanks!

Stephen