Re: Command Triggers

Hi,

Attached is a first version of the patch.


On Sunday, December 04, 2011 05:34:44 PM Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I have two questions now:
> > 
> > First, does anybody think it would be worth getting rid of the
> > duplication from OpenIntoRel (formerly from execMain.c) in regard to
> > DefineRelation()?
> 
> That's probably reasonable to do, since as you say it would remove the
> opportunity for bugs-of-omission in the CTAS table creation step.
> OTOH, if you find yourself having to make any significant changes to
> DefineRelation, then maybe not.
Building a CreateStmt seems to work well enough so far.

The only problem with that approach so far that I found is that:

CREATE TABLE collate_test2 (                                                                           
    a int,
    b text COLLATE "POSIX"
);

CREATE TABLE collate_test1 (                                                                           
    a int,
    b text COLLATE "C" NOT NULL
);

CREATE TABLE test_u AS SELECT a, b FROM collate_test1 UNION ALL SELECT a, b 
FROM collate_test2; -- fail

failed with:
ERROR:  no collation was derived for column "b" with collatable type text
HINT:  Use the COLLATE clause to set the collation explicitly.

"works" now.

I am currently setting ColumnDef.collOid of new collumns to attcollation of 
the QueryDesc's column. Unfortunately they have a different meaning...


> > Secondly, I am currently wondering whether it would be a good idea to use
> > the ModifyTable infrastructure for doing the insertion instead an own
> > DestReceiver infrastructure thats only used for CTAS.
> I think this is probably a bad idea; it will complicate matters and buy
> little.  There's not a lot of stuff needed for the actual data insertion
> step, since we know the table can't have any defaults, constraints,
> triggers, etc as yet.
I got to the same conclusion.


Remaining problems are:
* how to tell ExecContextForcesOids which oid we want
* implementing CREATE TABLE AS ... EXECUTE without duplicating ExecuteQuery
* the attcollation setting problems from above
* returning better error messages for using INTO at places its not allowed

Comments about the direction of the patch?


Andres