From: | NikhilS <nikkhils(at)gmail(dot)com> |
---|---|
To: | "Neil Conway" <neilc(at)samurai(dot)com> |
Cc: | "Bruce Momjian" <bruce(at)momjian(dot)us>, "Trevor Hardcastle" <chizu(at)spicious(dot)com>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: CREATE TABLE LIKE INCLUDING INDEXES support |
Date: | 2007-06-03 13:57:15 |
Message-ID: | d3c4af540706030657g49bda0bft3a05c02a1f72524d@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
Hi,
On 6/3/07, Neil Conway <neilc(at)samurai(dot)com> 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
From | Date | Subject | |
---|---|---|---|
Next Message | NikhilS | 2007-06-03 14:02:58 | Re: CREATE TABLE LIKE INCLUDING INDEXES support |
Previous Message | Magnus Hagander | 2007-06-03 08:51:03 | Re: syslogger line-end processing infelicity |
From | Date | Subject | |
---|---|---|---|
Next Message | NikhilS | 2007-06-03 14:02:58 | Re: CREATE TABLE LIKE INCLUDING INDEXES support |
Previous Message | Tom Lane | 2007-06-02 21:43:34 | Re: CREATE TABLE LIKE INCLUDING INDEXES support |