Re: BUG #6734: create table (like ...) fails if an index has a comment

On Fri, Jul 13, 2012 at 2:24 PM, Ryan Kelly <rpkelly22(at)gmail(dot)com> wrote:
> On Fri, Jul 13, 2012 at 12:00:14PM +0000, daniele(dot)varrazzo(at)gmail(dot)com wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference: 6734
>> Logged by: Daniele Varrazzo
>> Email address: daniele(dot)varrazzo(at)gmail(dot)com
>> PostgreSQL version: 9.1.4
>> Operating system: Linux
>> Description:
>>
>> Weird, isn't it? Test case below.
>>
>> Dropping the comment, the create table command works as expected. The
>> command fails with an: "ERROR: relation "foo2_f1_idx" already exists".
> The comments on chooseIndexName in src/backend/parser/parse_utilcmd.c say:
>
> * XXX this is inherently broken because the indexes aren't created
> * immediately, so we fail to resolve conflicts when the same name is
> * derived for multiple indexes.
>
> Which looks like the case here. So it seems like
> chooseIndexName/ChooseIndexName might need to take a list of any indexes
> names that have already been created to avoid this.

For the work I'm doing now (a migration of a table with a dozen of
indexes, that will need further process downstream) I'm finding it
would be much better to have name generation more predictable: even
without the bug, the index names are not very descriptive. Yes, they
contain the field name and the new table name, but the name may have
been something meaningful such as "open_contracts_idx". For
conflicting names (such as the ones in the test case, without the
comment) I guess it's undefined, or arbitrary anyway, which one gets
the numeric suffix.

Wouldn't it be better to call the indexes NEWTABLE_OLDINDEXNAME? They
would be more verbose, but generation of two equally named indexes in
the same table would be impossible (as OLDINDEXNAMEs are distinct) and
it would be easy to map old indexes into new ones. The "add a number
suffix" behavior would be still required to avoid conflict with the
name of an index of another table, but I guess this is already handled
by the current implementation.

-- Daniele