Re: ALTER TABLE does not check for column existence before startingoperations - Mailing list pgsql-hackers

From Arthur Zakirov
Subject Re: ALTER TABLE does not check for column existence before startingoperations
Date
Msg-id 20180626155917.GA10612@zakirov.localdomain
Whole thread Raw
In response to ALTER TABLE does not check for column existence before starting operations  (Pierre Ducroquet <pierre.ducroquet@people-doc.com>)
List pgsql-hackers
Hello,

On Fri, Mar 02, 2018 at 12:36:41PM +0100, Pierre Ducroquet wrote:
> The attached patch fixes this behaviour by adding a small check in the first 
> pass of alter table to make sure that a column referenced by an alter command 
> exists first. It also checks if the column is added by another alter sub-
> command. It does not handle every scenario (dropping a column and then 
> altering it for instance), these are left to the exec code to exclude.
> The patch has been checked with make check, and I see no documentation change 
> to do since this does not alter any existing documented behaviour.

I looked at the patch. It is true that there is no need to change the
documentation. Tests also passes (but maybe some changes would be
needed).

I have a couple comments:

> tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);

I think it is necessary to release the heap tuple using heap_freetuple()
if it is valid after all work done.

Second comment is related with several subcommands (ALTER COLUMN
DEFAULT, SET NOT NULL, SET/RESET (options)). The following query fails
with the patch:

=# alter table test
   alter non_existing set not null,
   add non_existing text;

It raises the error 'column "non_existing" of relation "test" does not
exist'. But without the patch the query is executed without errors. It
is because of how Phase 2 is performed. Subcommands are executed in a pass
determined by subcommand type. AT_PASS_ADD_COL subcommands are executed
before AT_PASS_ADD_INDEX, AT_PASS_ADD_CONSTR and AT_PASS_MISC.

I'm not sure how important it is. But I think it could break backward
compatibility.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Small fixes about backup history file in doc and pg_standby
Next
From: Pavel Stehule
Date:
Subject: Re: Unexpected behavior of DROP VIEW/TABLE IF EXISTS