Re: Command Triggers, patch v11

On Friday, March 16, 2012 10:52:55 PM Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On Friday, March 16, 2012 10:31:57 PM Tom Lane wrote:
> >> I'm thinking that if the table creation
> >> were to be moved into the tuple receiver's startup routine, we could
> >> avoid needing to get control back between ExecutorStartup and
> >> ExecutorRun, and then all that would be required would be to call
> >> ExecuteQuery with a different DestReceiver.
> >
> > Hm. I seriously dislike doing that in the receiver. I can't really point
> > out why though. Unsurprisingly I like getting the control back to
> > CreateTableAs...
>
> I don't see the argument. Receiver startup functions are intended to do
> exactly this type of thing. I'd be okay with leaving it in
> CreateTableAs if it didn't contort the code to do so, but what we have
> here is precisely that we've got to contort the interface with prepare.c
> to do it that way.
Hm.

> (It also occurs to me that moving this work into the DestReceiver might
> make things self-contained enough that we could continue to support
> EXPLAIN SELECT INTO with not an undue amount of pain.)

> Something else I just came across is that there are assorted places that
> are aware that ExplainStmt contains a Query, eg setrefs.c, plancache.c,
> and those have got to treat CreateTableAsStmt similarly.
Hm. Is that so? As implemented in my version the planner just sees a plain
statement instead of a utility statement. Am I missing something?

I am not even sure why the planner ever needs to see ExplainStmts? Protocol
level prepares? Shouldn't those top level nodes be simply removed once?

> We could just
> add more code in each of those places. I'm wondering though if it would
> be a good idea to invent an abstraction layer, to wit a utility.c
> function along the lines of "Query *UtilityContainsQuery(Node
> *parsetree)", which would centralize the knowledge of exactly which
> utility statements are like this and how to dig the Query out of them.
> It's only marginally attractive unless there's a foreseeable reason
> why we'd someday have more than two of them; but on the other hand,
> just formalizing the concept that some utility statements are like
> this might be a good thing.
If its really necessary to do that I think that would be a good idea. Alone
the increased greppablility/readablility seems to be worth it.

> (Actually, as I type this I wonder whether
> COPY (SELECT ...) isn't a member of this class too, and whether we don't
> have bugs from the fact that it's not being treated like EXPLAIN.)
> Thoughts?
Hm. On a first glance the planner also never sees the content of a CopyStmt...

Andres