From 9cebe49524af365d48f9e63048c79f537d6b135c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 15 Nov 2020 16:10:48 -0500 Subject: [PATCH] Fix fuzzy thinking about amcanmulticol versus amcaninclude. These flags should be independent: in particular an index AM should be able to say that it supports include columns without necessarily supporting multiple key columns. The included-columns patch got this wrong, possibly aided by the fact that it didn't bother to update the documentation. While here, clarify some text about amcanreturn, which was a little vague about what should happen when amcanreturn reports that only some of the index columns are returnable. Noted while reviewing the SP-GiST included-columns patch, which quite incorrectly (and unsafely) changed SP-GiST to claim amcanmulticol = true as a workaround for this bug. Backpatch to v11 where included columns were introduced. --- doc/src/sgml/indexam.sgml | 35 ++++++++++++++++++++++++-------- src/backend/commands/indexcmds.c | 4 ++-- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index da3d423abd..843fa1dcf4 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -181,7 +181,7 @@ typedef struct IndexAmRoutine implications. The requirements of amcanunique are discussed in . The amcanmulticol flag asserts that the - access method supports multicolumn indexes, while + access method supports multi-key-column indexes, while amoptionalkey asserts that it allows scans where no indexable restriction clause is given for the first index column. When amcanmulticol is false, @@ -217,6 +217,19 @@ typedef struct IndexAmRoutine conditions. + + The amcaninclude flag indicates whether the + access method supports included columns, that is it can + store (without processing) additional columns beyond the key column(s). + The requirements of the preceding paragraph apply only to the key + columns. In particular, the combination + of amcanmulticol=false + and amcaninclude=true is + sensible: it means that there can only be one key column, but there can + also be included column(s). Also, included columns must be allowed to be + null, independently of amoptionalkey. + + @@ -368,10 +381,13 @@ amcanreturn (Relation indexRelation, int attno); Check whether the index can support index-only scans on - the given column, by returning the indexed column values for an index entry - in the form of an IndexTuple. The attribute number - is 1-based, i.e., the first column's attno is 1. Returns true if supported, - else false. If the access method does not support index-only scans at all, + the given column, by returning the column's original indexed value. + The attribute number is 1-based, i.e., the first column's attno is 1. + Returns true if supported, else false. + This function should always return true for included columns + (if those are supported), since there's little point in an included + column that can't be retrieved. + If the access method does not support index-only scans at all, the amcanreturn field in its IndexAmRoutine struct can be set to NULL. @@ -461,7 +477,7 @@ amproperty (Oid index_oid, int attno, core code does not know how to do that and will return NULL. It may also be advantageous to implement AMPROP_RETURNABLE testing, if that can be done more cheaply than by opening the index and calling - amcanreturn, which is the core code's default behavior. + amcanreturn, which is the core code's default behavior. The default behavior should be satisfactory for all other standard properties. @@ -553,10 +569,13 @@ amgettuple (IndexScanDesc scan, If the index supports index-only - scans (i.e., amcanreturn returns true for it), + scans (i.e., amcanreturn returns true for any + of its columns), then on success the AM must also check scan->xs_want_itup, and if that is true it must return the originally indexed data for the - index entry. The data can be returned in the form of an + index entry. Columns for which amcanreturn returns + false can be returned as nulls. + The data can be returned in the form of an IndexTuple pointer stored at scan->xs_itup, with tuple descriptor scan->xs_itupdesc; or in the form of a HeapTuple pointer stored at scan->xs_hitup, diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 3424954594..7a96fdc1fc 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -402,7 +402,7 @@ DefineIndex(Oid relationId, list_copy(stmt->indexIncludingParams)); numberOfAttributes = list_length(allIndexParams); - if (numberOfAttributes <= 0) + if (numberOfKeyAttributes <= 0) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("must specify at least one column"))); @@ -618,7 +618,7 @@ DefineIndex(Oid relationId, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("access method \"%s\" does not support included columns", accessMethodName))); - if (numberOfAttributes > 1 && !amRoutine->amcanmulticol) + if (numberOfKeyAttributes > 1 && !amRoutine->amcanmulticol) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("access method \"%s\" does not support multicolumn indexes",