On Thu, Jul 4, 2013 at 7:32 AM, Dimitri Fontaine <dimitri@2ndquadrant.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
