Re: BUG #18637: CREATE INDEX won't look up operator classes in search_path if PARTITION BY is specified

I wrote:
> So that's pretty awful: creating the partitioned index by itself is
> willing to look up the opclass in the current search path, and then
> adding a new partition will play nice with that, but creating a
> partitioned index when there's already a partition will not.
> It's got to be considered a bug that the two paths for making a
> child index behave differently.

I looked into this and found that

* When adding a partition to a pre-existing partitioned index,
we use generateClonedIndexStmt() to construct the IndexStmt
for DefineIndex to use. This is proof against the current
problem because it will always schema-qualify the opclass
name, cf. get_opclass().

* However, if CREATE INDEX recurses to create a child partition,
it does this:

IndexStmt *childStmt = copyObject(stmt);

followed by some pretty ad-hoc cleanup. If the original
IndexStmt wasn't written in a search-path-independent fashion
then we've got trouble.

It seems clear to me that the right fix for this is to use
generateClonedIndexStmt in this code path too. I am not
claiming that generateClonedIndexStmt is entirely free of
related issues, but to the extent it has any we'd have to
fix them anyway. Building the IndexStmt in two fundamentally
different ways is just asking for trouble.

I hacked this up and was amused to discover that it also
fixes a TODO left over from c01eb619a: if there's a comment
for the parent index, we've been incorrectly applying it
to children too. generateClonedIndexStmt knows better than
to fill idxcomment for the child IndexStmt, but copyObject
does not.

This could stand an additional regression test case perhaps,
but the core tests don't have a suitable opclass at hand.
Maybe we could test it in some contrib module, but that
seems a shade hacky.

Thoughts?

regards, tom lane