Skip to content

Commit 5c5a268

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 81b052c commit 5c5a268

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
@@ -348,6 +348,14 @@ ConstructTupleDescriptor(Relation heapRelation,
348348
*/
349349
memcpy(to, from, ATTRIBUTE_FIXED_PART_SIZE);
350350

351+
/*
352+
* Set the attribute name as specified by caller.
353+
*/
354+
if (colnames_item == NULL) /* shouldn't happen */
355+
elog(ERROR, "too few entries in colnames list");
356+
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
357+
colnames_item = lnext(colnames_item);
358+
351359
/*
352360
* Fix the stuff that should not be the same as the underlying
353361
* 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
@@ -2379,6 +2379,23 @@ ERROR: duplicate key value violates unique constraint "func_index_index"
23792379
DETAIL: Key (textcat(f1, f2))=(ABCDEF) already exists.
23802380
-- but this shouldn't:
23812381
INSERT INTO func_index_heap VALUES('QWERTY');
2382+
-- while we're here, see that the metadata looks sane
2383+
\d func_index_heap
2384+
Table "public.func_index_heap"
2385+
Column | Type | Collation | Nullable | Default
2386+
--------+------+-----------+----------+---------
2387+
f1 | text | | |
2388+
f2 | text | | |
2389+
Indexes:
2390+
"func_index_index" UNIQUE, btree (textcat(f1, f2))
2391+
2392+
\d func_index_index
2393+
Index "public.func_index_index"
2394+
Column | Type | Definition
2395+
---------+------+-----------------
2396+
textcat | text | textcat(f1, f2)
2397+
unique, btree, for table "public.func_index_heap"
2398+
23822399
--
23832400
-- Same test, expressional index
23842401
--
@@ -2394,6 +2411,26 @@ ERROR: duplicate key value violates unique constraint "func_index_index"
23942411
DETAIL: Key ((f1 || f2))=(ABCDEF) already exists.
23952412
-- but this shouldn't:
23962413
INSERT INTO func_index_heap VALUES('QWERTY');
2414+
-- while we're here, see that the metadata looks sane
2415+
\d func_index_heap
2416+
Table "public.func_index_heap"
2417+
Column | Type | Collation | Nullable | Default
2418+
--------+------+-----------+----------+---------
2419+
f1 | text | | |
2420+
f2 | text | | |
2421+
Indexes:
2422+
"func_index_index" UNIQUE, btree ((f1 || f2))
2423+
2424+
\d func_index_index
2425+
Index "public.func_index_index"
2426+
Column | Type | Definition
2427+
--------+------+------------
2428+
expr | text | (f1 || f2)
2429+
unique, btree, for table "public.func_index_heap"
2430+
2431+
-- this should fail because of unsafe column type (anonymous record)
2432+
create index on func_index_heap ((f1 || f2), (row(f1, f2)));
2433+
ERROR: column "row" has pseudo-type record
23972434
--
23982435
-- Also try building functional, expressional, and partial indexes on
23992436
-- 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)