Further improve the names generated for indexes on expressions.

Commit 181b6185c failed to do anything useful with a whole-row Var,
deeming it "fishy".  But it is legal to put such a Var into an
expression index column, so let's expand it as the name of the table.

Another problem reachable via that one is that we could generate an
empty index column name, which isn't really legal although by chance
nothing complained about it.  It's not clear whether any other such
cases remain, but as cheap insurance let's use "expr" if the tree walk
fails to generate any text.

Reported-by: Chauhan Dhruv <chauhandhruv351@gmail.com>
Author: Chauhan Dhruv <chauhandhruv351@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CANWwWcp_DCJjq8pomeqp6W=fbygvzXXQO028VDJ9_6sLPjQnVA@mail.gmail.com

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 713bb5d10f19f55ba6566e3e01815da22ee7c18b..b71e588a953f8bd822d41b9e371891361ed95cd4 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2874,6 +2874,9 @@ ChooseIndexExpressionName(Relation rel, Node *indexExpr)
        context.buf = &buf;
        /* Walk the tree, stopping when we have enough text */
        (void) ChooseIndexExpressionName_walker(indexExpr, &context);
+       /* Fall back to "expr" if the walk found nothing, to avoid an empty name */
+       if (buf.len == 0)
+               appendStringInfoString(&buf, "expr");
        /* Ensure generated names are shorter than NAMEDATALEN */
        nlen = pg_mbcliplen(buf.data, buf.len, NAMEDATALEN - 1);
        buf.data[nlen] = '\0';
@@ -2891,19 +2894,29 @@ ChooseIndexExpressionName_walker(Node *node,
        {
                Var                *var = (Var *) node;
                TupleDesc       tupdesc = RelationGetDescr(context->rel);
-               Form_pg_attribute att;
+               const char *varname;
 
                /* Paranoia: ignore the Var if it looks fishy */
                if (var->varno != 1 || var->varlevelsup != 0 ||
-                       var->varattno <= 0 || var->varattno > tupdesc->natts)
+                       var->varattno < 0 || var->varattno > tupdesc->natts)
                        return false;
-               att = TupleDescAttr(tupdesc, var->varattno - 1);
-               if (att->attisdropped)
-                       return false;           /* even more paranoia; shouldn't happen */
+               if (var->varattno > 0)
+               {
+                       Form_pg_attribute att = TupleDescAttr(tupdesc, var->varattno - 1);
+
+                       if (att->attisdropped)
+                               return false;   /* even more paranoia; shouldn't happen */
+                       varname = NameStr(att->attname);
+               }
+               else
+               {
+                       /* Whole-row Var: use the table's name */
+                       varname = RelationGetRelationName(context->rel);
+               }
 
                if (context->buf->len > 0)
                        appendStringInfoChar(context->buf, '_');
-               appendStringInfoString(context->buf, NameStr(att->attname));
+               appendStringInfoString(context->buf, varname);
 
                /* Done if we've already reached NAMEDATALEN */
                return (context->buf->len >= NAMEDATALEN);

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 1145d57726ddaa2d29888146906d8947280994a4..7b2640f0e042da919fd325ea31d2025fa4b02695 100644 (file)
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1374,6 +1374,11 @@ ERROR:  duplicate key value violates unique constraint "func_index_index"
 DETAIL:  Key ((f1 || f2))=(ABCDEF) already exists.
 -- but this shouldn't:
 INSERT INTO func_index_heap VALUES('QWERTY');
+-- this should fail because of unsafe column type (anonymous record)
+create index on func_index_heap ((f1 || f2), (row(f1, f2)));
+ERROR:  column "f1_f21" has pseudo-type record
+-- but this is allowed:
+create index on func_index_heap ((func_index_heap.*));
 -- while we're here, see that the metadata looks sane
 \d func_index_heap
          Table "public.func_index_heap"
@@ -1382,6 +1387,7 @@ INSERT INTO func_index_heap VALUES('QWERTY');
  f1     | text |           |          | 
  f2     | text |           |          | 
 Indexes:
+    "func_index_heap_func_index_heap_idx" btree ((func_index_heap.*))
     "func_index_index" UNIQUE, btree ((f1 || f2))
 
 \d func_index_index
@@ -1391,9 +1397,13 @@ Indexes:
  f1_f2  | text | yes  | (f1 || f2)
 unique, btree, for table "public.func_index_heap"
 
--- this should fail because of unsafe column type (anonymous record)
-create index on func_index_heap ((f1 || f2), (row(f1, f2)));
-ERROR:  column "f1_f21" has pseudo-type record
+\d func_index_heap_func_index_heap_idx
+       Index "public.func_index_heap_func_index_heap_idx"
+     Column      |      Type       | Key? |     Definition      
+-----------------+-----------------+------+---------------------
+ func_index_heap | func_index_heap | yes  | (func_index_heap.*)
+btree, for table "public.func_index_heap"
+
 --
 -- Test unique index with included columns
 --

diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 8e59f6bcd01a8f445a6bf2496e36a868b570e241..88ca3c80875c424df51b2782bff08d38681b5731 100644 (file)
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -457,12 +457,16 @@ INSERT INTO func_index_heap VALUES('ABCD', 'EF');
 -- but this shouldn't:
 INSERT INTO func_index_heap VALUES('QWERTY');
 
+-- this should fail because of unsafe column type (anonymous record)
+create index on func_index_heap ((f1 || f2), (row(f1, f2)));
+
+-- but this is allowed:
+create index on func_index_heap ((func_index_heap.*));
+
 -- while we're here, see that the metadata looks sane
 \d func_index_heap
 \d func_index_index
-
--- this should fail because of unsafe column type (anonymous record)
-create index on func_index_heap ((f1 || f2), (row(f1, f2)));
+\d func_index_heap_func_index_heap_idx
 
 
 --