On 2015-07-03 15:50, Michael Paquier wrote:
> On Thu, Jul 2, 2015 at 11:16 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> I was going through the code and have few comments:
>> - Why do you change the return value of TryReuseIndex? Can't we use reuse
>> the same OidIsValid(stmt->oldNode) check that ATExecAddIndex is doing
>> instead?
>
> As pointed out by Heikki previously, that is actually unnecessary,
> comments are still lost even if the index is reused for constraints.
> So perhaps for simplicity we could just unconditionally recreate the
> comments all the time if they are available.
>
Ah ok, I missed Heikki's email.
>> - I think the changes to ATPostAlterTypeParse should follow more closely the
>> coding of transformTableLikeClause - namely use the idxcomment
>
> I am not sure I follow here. Could you elaborate?
>
Well for indexes you don't really need to add the new AT command, as
IndexStmt has char *idxcomment which it will automatically uses as
comment if not NULL. While I am not huge fan of the idxcomment it
doesn't seem to be easy to remove it in the future and it's what
transformTableLikeClause uses so it might be good to be consistent with
that.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
