On Wed, Apr 11, 2018 at 10:47 PM, Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> Teodor Sigaev wrote: > > > Patch attached. > > I wonder why this is a problem in opfamilies but not collations. > If we don't compare collations, wouldn't it make more sense to break out > of the loop once the number of keys is reached? > It appears that INCLUDE columns might have collation defined. For instance, following query is working: create index t_s1_s2_idx on t (s1) include (s2 collate "en_US.UTF-8"); However, I don't see any point in defining collations here, because INCLUDE attributes exist solely for index-only scans. So, index just can return value of INCLUDE attribute "as is", no point to do something with collation. So, I propose to disable collations for INCLUDE attributes. When this code was written, there was no question as to what length the > opfamilies/collations the arrays were; it was obvious that they must be > of the length of the index attributes. It's not obvious now. Maybe add > a comment about that? > In b3b7f789 Tom have resized one array size from total number of attributes to number of key attributes. And I like idea of applying the same to all other array: make them sized to total number of attributes with filling of absent attributes with 0. > > But it seems to me, field's names of > > IndexInfo structure are a bit confused now: > > int ii_NumIndexAttrs; /* total number of columns in index > */ > > int ii_NumIndexKeyAttrs; /* number of key columns in > index */ > > AttrNumber ii_KeyAttrNumbers[INDEX_MAX_KEYS]; > > > > ii_KeyAttrNumbers contains all columns, i.e. it contains ii_NumIndexAttrs > > number of columns, not a ii_NumIndexKeyAttrs number as easy to think. > > > > I suggest rename ii_KeyAttrNumbers to ii_AttrNumbers or > ii_IndexAttrNumbers. > > Opinions? > > Yeah, the current situation seems very odd. > +1 for ii_IndexAttrNumbers. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company