On Mon, 2007-21-05 at 12:23 +0530, NikhilS wrote:
> I had spent some time on this earlier so decided to complete and send
> the patch to you for review. This patch supports copying of
> expressions, predicates, opclass, amorder, reloptions etc. The test
> case also contains some more additions with this patch. Please let me
> know if there are any issues.
Attached is a revised version of this patch. Note that this pattern is
always unsafe:
ht_am = SearchSysCache(AMOID, ...);
if (!HeapTupleIsValid(ht_am))
elog(ERROR, "...");
amrec = (Form_pg_am) GETSTRUCT(ht_am);
index->accessMethod = NameStr(amrec->amname);
/* ... */
ReleaseSysCache(ht_am);
return index;
Before calling ReleaseSysCache(), all the data you need from the
syscache entry needs to be deep-copied to allow subsequent access, but
NameStr() doesn't do a deep-copy. Adding "-DFORCE_CATCACHE_RELEASE" is a
useful way to catch these kinds of problems (I wonder if this is worth
adding to the default CFLAGS when assertions are enabled?)
I also made a bunch of editorial changes, including moving the
varattnos_map_schema() call out of the loop in transformInhRelation().
BTW, comments like "This function is based on code from ruleutils.c"
would be helpful for reviewers (whether in the patch itself or just in
the email containing the patch).
There's still a few things I need to fix in the patch, but I'll apply a
revised version of the attached patch to HEAD tomorrow, barring any
objections.
-Neil
