On Friday, March 16, 2012 09:54:47 PM Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > [ ctas-01.patch ] > > I'm starting to look at this now. Great!
> For a patch that's supposed to de-complicate things, it seems pretty messy :-( Yea. It started out simple but never stopped getting more complicated. Getting rid of the duplication still seems to make sense to me though. > One thing I soon found is that it lacks support for EXPLAIN SELECT INTO. > That used to work, but now you get > > regression=# explain select * into foo from tenk1; > ERROR: INTO is only allowed on first SELECT of UNION/INTERSECT/EXCEPT > LINE 1: explain select * into foo from tenk1; > ^ > > and while fixing the parse analysis for that is probably not too hard, > it would still fail to work nicely, since explain.c lacks support for > CreateTableAsStmt utility statements.I think we'd have to invent > something parallel to ExplainExecuteQuery to support this, and I really > doubt that it's worth the trouble. Does anyone have a problem with > desupporting the case? I am all for removing it. > Also, it seems to me that the patch is spending way too much effort on > trying to exactly duplicate the old error messages for various flavors > of "INTO not allowed here", and still not succeeding terribly well. > I'm inclined to just have a one-size-fits-all message in > transformSelectStmt, which will fire if intoClause hasn't been cleared > before we get there; any objections? I was/am decidedly unhappy about the whole error message stuff. Thats what turned the patch from removing lines to making it bigger than before. I tried to be compatible to make sure I understood what was happening. I am fine with making it one simple error message. > A couple of other cosmetic thoughts: I'm tempted to split the execution > support out into a new file, rather than bloat tablecmds.c any further; > and I'm wondering if the interface to EXECUTE INTO can't be simplified. > (It may have been a mistake to remove the IntoClause in ExecuteStmt.) Hm. I vote against keeping the IntoClause stuff in ExecuteStmt. Splitting stuff from tablecmd.c seems sensible though. One more thing I disliked quite a bit was the duplication of the EXECUTE handling. Do you see a way to deduplicate that? If you give me some hints about what to change I am happy to revise the patch myself should you prefer that. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers