Re: [HACKERS] DROP COLUMN round 4

"Christopher Kings-Lynne" <chriskl(at)familyhealth(dot)com(dot)au> writes:
> Fixed. New diff attached - fixes regression tests as well, plus re-merged
> against HEAD.

I've applied this patch after some editorializing. Notably, I went
ahead and created syscache lookup convenience routines

extern HeapTuple SearchSysCacheAttName(Oid relid, const char *attname);
extern HeapTuple SearchSysCacheCopyAttName(Oid relid, const char *attname);
extern bool SearchSysCacheExistsAttName(Oid relid, const char *attname);

These are equivalent to the corresponding basic syscache routines for
the ATTNAME cache, except that they will release the cache entry and
return NULL (or false, etc) if the entry exists but has attisdropped =
true.

Essentially all uses of the ATTNAME cache should now go through these
routines instead. This cleans up a number of cases in which the patch
as-submitted produced rather bogus error messages.

I did not adopt the approach of inserting NULLs in eref lists, because
I'm afraid that will break code elsewhere that relies on eref lists.
Instead I added a post-facto check to scanRTEforColumn, which is simple
and reliable.

Also, I'm still unconvinced that there's any percentage in modifying the
name of a deleted column to avoid collisions, since it does nothing for
after-the-fact collisions; AFAICT the net result is just to *enlarge*
the set of column names that users have to avoid. So I took that code
out. A possible solution that would actually work is to replace
pg_attribute's unique index on (attrelid, attname) with one on
(attrelid, attname, attisdropped). However, I doubt that the problem is
worth the overhead that that would add. I'm inclined to leave the code
as it stands and just document that column names like
"........pg.dropped.N........" are best avoided.

There are still issues about functions that accept or return tuple
datatypes --- a DROP COLUMN is likely to break any functions that use
the table's tuple datatype. Related problems occur already with ADD
COLUMN, though, so I did not think this was a reason to reject the
patch.

regards, tom lane