Skip to content

Commit 275a8ac

Browse files
tglsfdctglsfdc
committed
Fix error reporting for index expressions of prohibited types.
If CheckAttributeType() threw an error about the datatype of an index expression column, it would report an empty column name, which is pretty unhelpful and certainly not the intended behavior. I (tgl) evidently broke this in commit cfc5008, by not noticing that the column's attname was used above where I'd placed the assignment of it. In HEAD and v12, this is trivially fixable by moving up the assignment of attname. Before v12 the code is a bit more messy; to avoid doing substantial refactoring, I took the lazy way out and just put in two copies of the assignment code. Report and patch by Amit Langote. Back-patch to all supported branches. Discussion: https://postgr.es/m/CA+HiwqFA+BGyBFimjiYXXMa2Hc3fcL0+OJOyzUNjhU4NCa_XXw@mail.gmail.com
1 parent 65cb25e commit 275a8ac

File tree

3 files changed

+65
-8
lines changed

3 files changed

+65
-8
lines changed

‎src/backend/catalog/index.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,14 @@ ConstructTupleDescriptor(Relation heapRelation,
349349
*/
350350
memcpy(to, from, ATTRIBUTE_FIXED_PART_SIZE);
351351

352+
/*
353+
* Set the attribute name as specified by caller.
354+
*/
355+
if (colnames_item == NULL) /* shouldn't happen */
356+
elog(ERROR, "too few entries in colnames list");
357+
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
358+
colnames_item = lnext(colnames_item);
359+
352360
/*
353361
* Fix the stuff that should not be the same as the underlying
354362
* attr
@@ -370,6 +378,14 @@ ConstructTupleDescriptor(Relation heapRelation,
370378

371379
MemSet(to, 0, ATTRIBUTE_FIXED_PART_SIZE);
372380

381+
/*
382+
* Set the attribute name as specified by caller.
383+
*/
384+
if (colnames_item == NULL) /* shouldn't happen */
385+
elog(ERROR, "too few entries in colnames list");
386+
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
387+
colnames_item = lnext(colnames_item);
388+
373389
if (indexpr_item == NULL) /* shouldn't happen */
374390
elog(ERROR, "too few entries in indexprs list");
375391
indexkey = (Node *) lfirst(indexpr_item);
@@ -422,14 +438,6 @@ ConstructTupleDescriptor(Relation heapRelation,
422438
*/
423439
to->attrelid = InvalidOid;
424440

425-
/*
426-
* Set the attribute name as specified by caller.
427-
*/
428-
if (colnames_item == NULL) /* shouldn't happen */
429-
elog(ERROR, "too few entries in colnames list");
430-
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
431-
colnames_item = lnext(colnames_item);
432-
433441
/*
434442
* Check the opclass and index AM to see if either provides a keytype
435443
* (overriding the attribute type). Opclass takes precedence.

‎src/test/regress/expected/create_index.out

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2384,6 +2384,23 @@ ERROR: duplicate key value violates unique constraint "func_index_index"
23842384
DETAIL: Key (textcat(f1, f2))=(ABCDEF) already exists.
23852385
-- but this shouldn't:
23862386
INSERT INTO func_index_heap VALUES('QWERTY');
2387+
-- while we're here, see that the metadata looks sane
2388+
\d func_index_heap
2389+
Table "public.func_index_heap"
2390+
Column | Type | Modifiers
2391+
--------+------+-----------
2392+
f1 | text |
2393+
f2 | text |
2394+
Indexes:
2395+
"func_index_index" UNIQUE, btree (textcat(f1, f2))
2396+
2397+
\d func_index_index
2398+
Index "public.func_index_index"
2399+
Column | Type | Definition
2400+
---------+------+-----------------
2401+
textcat | text | textcat(f1, f2)
2402+
unique, btree, for table "public.func_index_heap"
2403+
23872404
--
23882405
-- Same test, expressional index
23892406
--
@@ -2399,6 +2416,26 @@ ERROR: duplicate key value violates unique constraint "func_index_index"
23992416
DETAIL: Key ((f1 || f2))=(ABCDEF) already exists.
24002417
-- but this shouldn't:
24012418
INSERT INTO func_index_heap VALUES('QWERTY');
2419+
-- while we're here, see that the metadata looks sane
2420+
\d func_index_heap
2421+
Table "public.func_index_heap"
2422+
Column | Type | Modifiers
2423+
--------+------+-----------
2424+
f1 | text |
2425+
f2 | text |
2426+
Indexes:
2427+
"func_index_index" UNIQUE, btree ((f1 || f2))
2428+
2429+
\d func_index_index
2430+
Index "public.func_index_index"
2431+
Column | Type | Definition
2432+
--------+------+------------
2433+
expr | text | (f1 || f2)
2434+
unique, btree, for table "public.func_index_heap"
2435+
2436+
-- this should fail because of unsafe column type (anonymous record)
2437+
create index on func_index_heap ((f1 || f2), (row(f1, f2)));
2438+
ERROR: column "row" has pseudo-type record
24022439
--
24032440
-- Also try building functional, expressional, and partial indexes on
24042441
-- tables that already contain data.

‎src/test/regress/sql/create_index.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,10 @@ INSERT INTO func_index_heap VALUES('ABCD', 'EF');
715715
-- but this shouldn't:
716716
INSERT INTO func_index_heap VALUES('QWERTY');
717717

718+
-- while we're here, see that the metadata looks sane
719+
\d func_index_heap
720+
\d func_index_index
721+
718722

719723
--
720724
-- Same test, expressional index
@@ -731,6 +735,14 @@ INSERT INTO func_index_heap VALUES('ABCD', 'EF');
731735
-- but this shouldn't:
732736
INSERT INTO func_index_heap VALUES('QWERTY');
733737

738+
-- while we're here, see that the metadata looks sane
739+
\d func_index_heap
740+
\d func_index_index
741+
742+
-- this should fail because of unsafe column type (anonymous record)
743+
create index on func_index_heap ((f1 || f2), (row(f1, f2)));
744+
745+
734746
--
735747
-- Also try building functional, expressional, and partial indexes on
736748
-- tables that already contain data.

0 commit comments

Comments
 (0)