pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org, Thom Brown <thom@linux.com>
Cc: Dimitri Fontaine <dimitri@2ndquadrant.fr>
Subject: Re: Command Triggers, patch v11
Date: Fri, 16 Mar 2012 16:54:47 -0400
Message-ID: <20905.1331931287@sss.pgh.pa.us> (raw)
In-Reply-To: <201203041650.39826.andres@anarazel.de>
References: <m24nufq466.fsf@2ndQuadrant.fr>
	<CAA-aLv4kv3DXuwXdDyU2qcWLjvYCsVo5=qTZq+t8uk5S9sAkqQ@mail.gmail.com>
	<CAA-aLv5paBN4P-cXUsu=KMQva-SCSjo9MpzM6szYFt5wqeFkeg@mail.gmail.com>
	<201203041650.39826.andres@anarazel.de>

Andres Freund <andres@anarazel.de> writes:
> [ ctas-01.patch ]

I'm starting to look at this now.  For a patch that's supposed to
de-complicate things, it seems pretty messy :-(

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?

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?

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.)

			regards, tom lane



view thread (115+ messages)  latest in thread

Message-ID: <20905.1331931287@sss.pgh.pa.us>
Permalink:  ../20905.1331931287@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/20905.1331931287@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, thom@linux.com, dimitri@2ndquadrant.fr
  Subject: Re: Command Triggers, patch v11
  In-Reply-To: <20905.1331931287@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox