Skip to content

Commit 298d056

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 cfb2a4c commit 298d056

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
@@ -343,6 +343,14 @@ ConstructTupleDescriptor(Relation heapRelation,
343343
*/
344344
memcpy(to, from, ATTRIBUTE_FIXED_PART_SIZE);
345345

346+
/*
347+
* Set the attribute name as specified by caller.
348+
*/
349+
if (colnames_item == NULL) /* shouldn't happen */
350+
elog(ERROR, "too few entries in colnames list");
351+
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
352+
colnames_item = lnext(colnames_item);
353+
346354
/*
347355
* Fix the stuff that should not be the same as the underlying
348356
* attr
@@ -364,6 +372,14 @@ ConstructTupleDescriptor(Relation heapRelation,
364372

365373
MemSet(to, 0, ATTRIBUTE_FIXED_PART_SIZE);
366374

375+
/*
376+
* Set the attribute name as specified by caller.
377+
*/
378+
if (colnames_item == NULL) /* shouldn't happen */
379+
elog(ERROR, "too few entries in colnames list");
380+
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
381+
colnames_item = lnext(colnames_item);
382+
367383
if (indexpr_item == NULL) /* shouldn't happen */
368384
elog(ERROR, "too few entries in indexprs list");
369385
indexkey = (Node *) lfirst(indexpr_item);
@@ -416,14 +432,6 @@ ConstructTupleDescriptor(Relation heapRelation,
416432
*/
417433
to->attrelid = InvalidOid;
418434

419-
/*
420-
* Set the attribute name as specified by caller.
421-
*/
422-
if (colnames_item == NULL) /* shouldn't happen */
423-
elog(ERROR, "too few entries in colnames list");
424-
namestrcpy(&to->attname, (const char *) lfirst(colnames_item));
425-
colnames_item = lnext(colnames_item);
426-
427435
/*
428436
* Check the opclass and index AM to see if either provides a keytype
429437
* (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
@@ -2256,6 +2256,23 @@ ERROR: duplicate key value violates unique constraint "func_index_index"
22562256
DETAIL: Key (textcat(f1, f2))=(ABCDEF) already exists.
22572257
-- but this shouldn't:
22582258
INSERT INTO func_index_heap VALUES('QWERTY');
2259+
-- while we're here, see that the metadata looks sane
2260+
\d func_index_heap
2261+
Table "public.func_index_heap"
2262+
Column | Type | Modifiers
2263+
--------+------+-----------
2264+
f1 | text |
2265+
f2 | text |
2266+
Indexes:
2267+
"func_index_index" UNIQUE, btree (textcat(f1, f2))
2268+
2269+
\d func_index_index
2270+
Index "public.func_index_index"
2271+
Column | Type | Definition
2272+
---------+------+-----------------
2273+
textcat | text | textcat(f1, f2)
2274+
unique, btree, for table "public.func_index_heap"
2275+
22592276
--
22602277
-- Same test, expressional index
22612278
--
@@ -2271,6 +2288,26 @@ ERROR: duplicate key value violates unique constraint "func_index_index"
22712288
DETAIL: Key ((f1 || f2))=(ABCDEF) already exists.
22722289
-- but this shouldn't:
22732290
INSERT INTO func_index_heap VALUES('QWERTY');
2291+
-- while we're here, see that the metadata looks sane
2292+
\d func_index_heap
2293+
Table "public.func_index_heap"
2294+
Column | Type | Modifiers
2295+
--------+------+-----------
2296+
f1 | text |
2297+
f2 | text |
2298+
Indexes:
2299+
"func_index_index" UNIQUE, btree ((f1 || f2))
2300+
2301+
\d func_index_index
2302+
Index "public.func_index_index"
2303+
Column | Type | Definition
2304+
--------+------+------------
2305+
expr | text | (f1 || f2)
2306+
unique, btree, for table "public.func_index_heap"
2307+
2308+
-- this should fail because of unsafe column type (anonymous record)
2309+
create index on func_index_heap ((f1 || f2), (row(f1, f2)));
2310+
ERROR: column "row" has pseudo-type record
22742311
--
22752312
-- Also try building functional, expressional, and partial indexes on
22762313
-- 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
@@ -678,6 +678,10 @@ INSERT INTO func_index_heap VALUES('ABCD', 'EF');
678678
-- but this shouldn't:
679679
INSERT INTO func_index_heap VALUES('QWERTY');
680680

681+
-- while we're here, see that the metadata looks sane
682+
\d func_index_heap
683+
\d func_index_index
684+
681685

682686
--
683687
-- Same test, expressional index
@@ -694,6 +698,14 @@ INSERT INTO func_index_heap VALUES('ABCD', 'EF');
694698
-- but this shouldn't:
695699
INSERT INTO func_index_heap VALUES('QWERTY');
696700

701+
-- while we're here, see that the metadata looks sane
702+
\d func_index_heap
703+
\d func_index_index
704+
705+
-- this should fail because of unsafe column type (anonymous record)
706+
create index on func_index_heap ((f1 || f2), (row(f1, f2)));
707+
708+
697709
--
698710
-- Also try building functional, expressional, and partial indexes on
699711
-- tables that already contain data.

0 commit comments

Comments
 (0)