Possible patch for better index name choosing

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Possible patch for better index name choosing
Date: 2009-12-21 03:17:17
Message-ID: 25441.1261365437@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attached is a WIP patch for addressing the problems mentioned in this
thread:
http://archives.postgresql.org/pgsql-hackers/2009-12/msg01764.php

The main things that it does are (1) consider all index columns, not
just the first one as formerly; and (2) try to generate a usable name
for index expression columns, rather than just ignoring them which was
the effective behavior formerly.

There are several changes in the regression test outputs, mostly as a
result of choice (1). I've not bothered to update the expected files
yet but just attached the output diffs to show what happens.

There is one thing that is not terribly nice about the behavior, which
is that CREATE TABLE LIKE INCLUDING INDEXES is unable to generate smart
names for expression indexes; it falls back to "expr", as for example
in

regression=# create table foo (f1 text, exclude (lower(f1) with =));
NOTICE: CREATE TABLE / EXCLUDE will create implicit index "foo_lower_exclusion" for table "foo"
CREATE TABLE
regression=# create table foo2 (like foo including all);
NOTICE: CREATE TABLE / EXCLUDE will create implicit index "foo2_expr_exclusion" for table "foo2"
CREATE TABLE

The reason for this is that the patch depends on FigureColname which
works on untransformed parse trees, and we don't have access to such
a tree when copying an existing index. There seem to be three possible
responses to that:

1. Decide this isn't worth worrying about and use the patch as-is.

2. Change FigureColname to work on already-transformed expressions.
I don't care for this idea much, for two reasons. First, FigureColname
would become significantly slower (eg, it would have to do catalog
lookups to resolve names of Vars, instead of just pulling the name out
of a ColumnRef), and this is objectionable considering it's part of
the required parsing path for even very simple commands. Second, there
are various corner cases where we'd get different results, which would
likely break applications that are expecting specific result column
names from given queries.

3. Implement a separate FigureIndexColname function that works as much
like FigureColname as it can, but takes a transformed parse tree.
This fixes the LIKE case and also removes the need for the iexprname
field that the attached patch adds to IndexElem. I think it largely
overcomes the two objections to idea #2, since an extra few lookups
during index creation are hardly a performance problem, and exact
application compatibility shouldn't be an issue here either. It's
a bit ugly to have to keep two such functions in sync though.

I'm not real sure whether to go with the patch as-is or use idea #3.
It seems to depend on how annoyed you are by the LIKE behavior.

A different consideration is whether it's really a good idea to be
messing with default index names at all. As illustrated in the attached
regression diffs, this does impact the error messages returned to
applications for unique-index failures. I don't think this is a serious
problem across a major version update, but maybe someone thinks
differently.

Comments?

regards, tom lane

Attachment Content-Type Size
index-naming-1.patch text/x-patch 23.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2009-12-21 03:23:57 Re: Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts
Previous Message Andres Freund 2009-12-21 03:02:37 Small Bug in GetConflictingVirtualXIDs