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.
> - 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?
> - Also the changes to ATPostAlterTypeParse move the code somewhat
> uncomfortably over the 80 char width, I don't really see a way to fix that
> except for moving some of the nesting out to another function.
Yes, I did some refactoring here by adding a set of new routines
dedicated to attach generated commands to the correct queue. This way
the code is empty of large switch/case blocks.
Update patch is attached, with the issue reported by Heikki upthread
fixed as well.
Regards,
--
Michael