Hi, On 6/3/07, Neil Conway <[EMAIL PROTECTED]> wrote:
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 should have delved deep into the NameStr functionality myself for this. I too agree that adding the flag makes sense 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).
Yes I had this in mind but somehow forgot to mention this when I posted the patch. I had done some changes in ruleutils.c specificly to take cognizance of this fact. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com