From: | Jaime Casanova <jaime(at)2ndquadrant(dot)com> |
---|---|
To: | Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> |
Cc: | Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: in-catalog Extension Scripts and Control parameters (templates?) |
Date: | 2013-07-04 14:55:50 |
Message-ID: | CAJKUy5hyRSYybn=AtSfYVVDmdYzCTTgtL7+6Je0FfScm9g2dUQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jul 4, 2013 at 7:32 AM, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>
>> - In alter.c you made AlterObjectRename_internal non static and
>> replaced a SearchSysCache1 call with a get_catalog_object_by_oid one.
>> Now, in its comment that function says that is for simple cases. And
>> because of the things you're doing it seems to me this isn't a simple
>> case. Maybe instead of modifying it you should create other function
>> RenameExtensionTemplateInternal, just like tablecmd.c does?
>
> The get_catalog_object_by_oid() is doing a SearchSysCache1 when that's
> relevant and possible, so I don't think I changed enough things around
> to warrant a different API. I'm open to hearing about why I'm wrong if
> that's the case, though.
>
not sure if you're wrong. but at the very least, you miss a
heap_freetuple(oldtup) there, because get_catalog_object_by_oid()
>
>> - extension.c: In function ‘get_ext_ver_list_from_catalog’:
>> extension.c:1150:25: warning: variable ‘evi’ set but not used
>> [-Wunused-but-set-variable]
>
> I don't have the warning here, and that code is unchanged from master's
> branch, only the name of the function did change. Do you have the same
> warning with master? which version of gcc are you using?
>
no, that code is not unchanged because function
get_ext_ver_list_from_catalog() comes from your patch.
it's seems the thing is that function get_ext_ver_info() is append a
new ExtensionVersionInfo which is then returned and assigned to an
*evi pointer that is never used.
i'm sure that evi in line 1150 is only because you need to receive the
returned value. Maybe you could use "(void) get_ext_ver_info()" (or it
should be (void *)?) to avoid that assignment and keep compiler quiet
PS: i'm on gcc (Debian 4.7.2-5) 4.7.2
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2013-07-04 15:13:42 | Re: Mention in bgworker docs that db connection needs shmem access |
Previous Message | Peter Eisentraut | 2013-07-04 14:29:10 | Re: event trigger API documentation? |