Possible patch for better index name choosing
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@postgreSQL.org
Date: 2009-12-21T03:17:17Z
Lists: pgsql-hackers
Attachments
- index-naming-1.patch (text/x-patch) patch
- (unnamed) (text/plain)
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