I noticed that this patch uses names for some things (for example it
gets the name of the access method), and then builds a IndexStmt which
contains the name.  I don't think this is a good idea.  I think what
should happen here is that the function to build indexes should be split
in two: one to resolve the names and fill a structure with Oids of
things, and another to get that structure and actually build the index
or constraint.  For example look into how GrantStmt is turned into
InternalGrant, and the stuff in aclchk.c to work with that.

The index creation happens after the new table (which is LIKE the parent)
has been created, by appending the cxt.alist information to "extras_after".
The entry point for the index creation is thus via ProcessUtility which
expects an IndexStmt structure. That is why the current patch does all the
Oid to name mapping exercise to populate the relevant fields in IndexStmt
some of which are char pointers. The internal DefineIndex function also
expects most of the fields to be in IndexStmt like format.

If we want to follow the above suggestion, as I understand it, we might have
to devise a new structure to contain Oids and make ProcessUtility accept a
new nodeTag. We will also not be able to use existing Index definition
functions and this will lead to more coding IMHO. Do we want to go down this
path? Or is there something else that has been suggested above and that I am
missing completely?

Please let me know.
EnterpriseDB               http://www.enterprisedb.com

Reply via email to