Re: WIP: Covering + unique indexes.

On Thu, Feb 16, 2017 at 6:43 PM, Anastasia Lubennikova
<a.lubennikova@postgrespro.ru> wrote:
> 14.02.2017 15:46, Amit Kapila:
>
>
>> 4.
>> + IDENTITY_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IMPORT_P IN_P
>> INCLUDE
>>    INCLUDING INCREMENT INDEX INDEXES INHERIT INHERITS INITIALLY INLINE_P
>>    INNER_P INOUT INPUT_P INSENSITIVE INSERT INSTEAD INT_P INTEGER
>>    INTERSECT INTERVAL INTO INVOKER IS ISNULL ISOLATION
>> @@ -3431,17 +3433,18 @@ ConstraintElem:
>>    n->initially_valid = !n->skip_validation;
>>    $$ = (Node *)n;
>>    }
>> - | UNIQUE '(' columnList ')' opt_definition OptConsTableSpace
>> + | UNIQUE '(' columnList ')' opt_c_including opt_definition
>> OptConsTableSpace
>>
>> If we want to use INCLUDE in syntax, then it might be better to keep
>> the naming reflect the same.  For ex. instead of opt_c_including we
>> should use opt_c_include.
>>
>
>
> Thank you for this suggestion. I've just wrote the code looking at examples
> around,
> but optincluding and optcincluding clauses seem to be redundant.
> I've cleaned up the code.
>

I think you have cleaned only in gram.y as I could see the references
to 'including' in other parts of code.  For ex, see below code:
@@ -2667,6 +2667,7 @@ _copyConstraint(const Constraint *from)
  COPY_NODE_FIELD(raw_expr);
  COPY_STRING_FIELD(cooked_expr);
  COPY_NODE_FIELD(keys);
+ COPY_NODE_FIELD(including);
  COPY_NODE_FIELD(exclusions);
  COPY_NODE_FIELD(options);
  COPY_STRING_FIELD(indexname);
@@ -3187,6 +3188,7 @@ _copyIndexStmt(const IndexStmt *from)
  COPY_STRING_FIELD(accessMethod);
  COPY_STRING_FIELD(tableSpace);
  COPY_NODE_FIELD(indexParams);
+ COPY_NODE_FIELD(indexIncludingParams);


@@ -425,6 +425,13 @@ ConstructTupleDescriptor(Relation heapRelation,
  /*
+ * Code below is concerned to the opclasses which are not used
+ * with the included columns.
+ */
+ if (i >= indexInfo->ii_NumIndexKeyAttrs)
+ continue;
+

There seems to be code below the above check which is not directly
related to opclasses, so not sure if you have missed that or is there
any other reason to ignore that.  I am referring to following code in
the same function after the above check:
/*
* If a key type different from the heap value is specified, update
*
the type-related fields in the index tupdesc.
*/
if (OidIsValid(keyType) &&
keyType != to->atttypid)


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com