Thread: LIKE INCLUDING COMMENTS code is a flight of fancy
I just got done fixing a different problem in that area, and then I
noticed this:
regression=# create table src (f1 text);
CREATE TABLE
regression=# create index srclower on src(lower(f1));
CREATE INDEX
regression=# comment on column srclower.pg_expression_1 is 'a comment';
COMMENT
regression=# create index srcupper on src(upper(f1));
CREATE INDEX
regression=# comment on column srcupper.pg_expression_1 is 'another comment';
COMMENT
regression=# create table dest (like src including all);
ERROR: relation "dest_key" already exists
The reason this fails is that the LIKE INCLUDING COMMENTS code thinks it
can use ChooseRelationName() in advance of actually creating the
indexes (cf. chooseIndexName in parse_utilcmd.c). This does not work.
That function is only meant to select a name for an index that will be
created immediately --- otherwise, its logic for avoiding collisions
does not work at all. As illustrated above.
I don't immediately see a fix for this that isn't a great deal more
trouble than the feature is worth. I suggest that we might want to just
rip out the support for copying comments on indexes. Or maybe even the
whole copy-comments aspect of it.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > I suggest that we might want to just > rip out the support for copying comments on indexes. Or maybe even the > whole copy-comments aspect of it. We have two related ToDo items below. They are a bit inconsintent, but they mean we should forbid COMMENT on columns of an index, or must have full-support of the feature. Which direction should we go? As for me, forbidding comments on index columns seems to be a saner way because index can have arbitrary key names in some cases. - Forbid COMMENT on columns of an index Postgres currently allows comments to be placed on the columns of an index, butpg_dump doesn't handle them and the column names themselves are implementation-dependent. http://archives.postgresql.org/message-id/27676.1237906577@sss.pgh.pa.us - pg_dump / pg_restore: Add dumping of comments on index columns and composite type columns http://archives.postgresql.org/pgsql-hackers/2009-03/msg00931.php (XXX: Comments on composite type columns can work now?) Regards, --- Takahiro Itagaki NTT Open Source Software Center
Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> writes: > Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I suggest that we might want to just >> rip out the support for copying comments on indexes. > We have two related ToDo items below. They are a bit inconsintent, > but they mean we should forbid COMMENT on columns of an index, > or must have full-support of the feature. > Which direction should we go? As for me, forbidding comments on index > columns seems to be a saner way because index can have arbitrary key > names in some cases. > - Forbid COMMENT on columns of an index > Postgres currently allows comments to be placed on the columns of > an index, but pg_dump doesn't handle them and the column names > themselves are implementation-dependent. > http://archives.postgresql.org/message-id/27676.1237906577@sss.pgh.pa.us > - pg_dump / pg_restore: Add dumping of comments on index columns and > composite type columns > http://archives.postgresql.org/pgsql-hackers/2009-03/msg00931.php > (XXX: Comments on composite type columns can work now?) I'm for forbidding comments on index columns. The amount of work required to support the feature fully seems far out of proportion to its value. In any case, if pg_dump drops such comments (which I had forgotten, but it seems true after a quick look at the code), then we could certainly get away with having LIKE not copy them. That would fix the immediate problem. regards, tom lane
Tom Lane wrote: > > I'm for forbidding comments on index columns. The amount of work > required to support the feature fully seems far out of proportion to > its value. > > In any case, if pg_dump drops such comments (which I had forgotten, > but it seems true after a quick look at the code), then we could > certainly get away with having LIKE not copy them. That would fix > the immediate problem. > > > If we're going to keep the comments we should dump them. I don't mind dropping them altogether - it's hardly a killer feature. We should just be consistent, IMNSHO. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> I'm for forbidding comments on index columns. The amount of work
>> required to support the feature fully seems far out of proportion to
>> its value.
>>
>> In any case, if pg_dump drops such comments (which I had forgotten,
>> but it seems true after a quick look at the code), then we could
>> certainly get away with having LIKE not copy them. That would fix
>> the immediate problem.
> If we're going to keep the comments we should dump them. I don't mind
> dropping them altogether - it's hardly a killer feature. We should just
> be consistent, IMNSHO.
Well, let's just forbid them then. It'll take just a few extra lines in
comment.c to throw an error if the target table has the wrong relkind.
Then we can pull out the troublesome code in parse_utilcmds.c.
It strikes me also that this changes the terms of discussion for the
other patch I was working on. I was mistakenly assuming that we could
not change the naming convention for individual index columns because
it would cause errors during dump/reload of per-column comments. But
since pg_dump never has supported that, there is no such risk. I
propose re-instating my previous idea of replacing "pg_expression_n"
with the name chosen by FigureColname. This not only makes the index
column names a bit more useful, but it fixes the disadvantage I was
on about for CREATE TABLE LIKE: it can get the FigureColname name
from the index's pg_attribute entry, so there's no problem with
producing smart index-expression column names when cloning indexes.
regards, tom lane