Re: in-catalog Extension Scripts and Control parameters (templates?)

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

In response to

Responses

Browse pgsql-hackers by date

  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?