Re: BUG #3403: ver 8.2 can't add serial column to temp table, but 8.1 can

Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
> 8.1 creates the sequence in wrong schema:

Yeah, 8.0 and 8.1 both do the wrong thing really, so it's hard to
argue that this ever worked.

> The problem seems to be in transformColumnDefinition, where the schema
> of the to-be-created sequence is determined from the relation name
> given. The default creation schema is used, if the user didn't specify
> the schame of the table explicitly, but since it's an ALTER TABLE, it
> really should use the schema of the existing table.

This is actually a bit nasty. Your proposed patch doesn't really work,
because of the concern that is now commented at the head of
transformAlterTableStmt:

* CAUTION: resist the temptation to do any work here that depends on the
* current state of the table. Actual execution of the command might not
* occur till some future transaction. Hence, we do only purely syntactic
* transformations here, comparable to the processing of CREATE TABLE.

IOW, we don't actually *know* at parse analysis time which table will be
affected.

It's arguable that CREATE TABLE with a serial is broken too, because
conceivably search_path could change between parsing and execution
of the command, leading to the table being created in the new default
schema while the sequence still goes into the old one.

It looks to me like a "proper" fix requires postponing the formation of
the CREATE SEQUENCE command until execution time, when we can know with
some confidence what schema the table is in. Yech. That'll be pretty
invasive ... is it worth the trouble?

A possible alternative is to interpret CREATE/ALTER TABLE as nailing
down the target schema at parse analysis time, ie, after analysis the
query always looks as if you had written an explicit schema name rather
than leaving it up to search_path. But this would be a behavioral
change that would likely bite somebody; and it would be inconsistent
with the behavior of other utility commands.

Maybe we should give up doing any CREATE/ALTER processing at all at
parse analysis time, and push it all to execution time. I got rid of
parse-time processing of other utility statements during the plan
caching work a couple months ago, because of concerns very much like
this, but I hadn't bit the bullet for CREATE/ALTER TABLE because it was
such a huge chunk of code. But maybe we'd better do it.

Comments?

regards, tom lane