On 2015-07-03 15:50, Michael Paquier wrote:
On Thu, Jul 2, 2015 at 11:16 PM, Petr Jelinek <p...@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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to