Re: pg_migrator issue with contrib
От | Merlin Moncure |
---|---|
Тема | Re: pg_migrator issue with contrib |
Дата | |
Msg-id | b42b73150906080610qeea303dq20f6598f3956ac19@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: pg_migrator issue with contrib (Tom Lane <tgl@sss.pgh.pa.us>) |
Список | pgsql-hackers |
On Sun, Jun 7, 2009 at 12:36 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: > I wrote: >> * pageinspect has changed the ABI of get_raw_page() in a way that will >> likely make it dump core if the function definition is migrated from >> an old DB. This needs to be fixed. >> [ and similarly for some other contrib modules ] > > After thinking about this some more, I think that there is a fairly > simple coding rule we can adopt to prevent post-migration crashes > of the sort I'm worrying about above. That is: > > * If you change the ABI of a C-language function, change its C name. > > This will ensure that if someone tries to migrate the old function > definition from an old database, they will get a pg_migrator failure, > or at worst a clean runtime failure when they attempt to use the old > definition. They won't get a core dump or some worse form of security > problem. > > As an example, the problem in pageinspect is this diff: > > *************** > *** 6,16 **** > -- > -- get_raw_page() > -- > ! CREATE OR REPLACE FUNCTION get_raw_page(text, int4) > RETURNS bytea > AS 'MODULE_PATHNAME', 'get_raw_page' > LANGUAGE C STRICT; > > -- > -- page_header() > -- > --- 6,21 ---- > -- > -- get_raw_page() > -- > ! CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4) > RETURNS bytea > AS 'MODULE_PATHNAME', 'get_raw_page' > LANGUAGE C STRICT; > > + CREATE OR REPLACE FUNCTION get_raw_page(text, int4) > + RETURNS bytea > + AS $$ SELECT get_raw_page($1, 'main', $2); $$ > + LANGUAGE SQL STRICT; > + > -- > -- page_header() > -- > *************** > > The underlying C-level get_raw_page function is still there, but > it now expects three arguments not two, and will crash if it's > passed an int4 where it's expecting a text argument. But the old > function definition will migrate without error --- there's no way > for pg_migrator to realize it's installing a security hazard. > > The way we should have done this, which I intend to go change it to, > is something like > > CREATE OR REPLACE FUNCTION get_raw_page(text, int4) > RETURNS bytea > AS 'MODULE_PATHNAME', 'get_raw_page' > LANGUAGE C STRICT; > > CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4) > RETURNS bytea > AS 'MODULE_PATHNAME', 'get_raw_page_3' > LANGUAGE C STRICT; > > so that the old function's ABI is preserved. Migration of the old > contrib module will then lead to the 3-argument function not being > immediately available, but the 2-argument one still works. Had we not > wanted to keep the 2-argument form for some reason, we would have > provided only get_raw_page_3 in the .so file, and attempts to use the > old function definition would fail safely. > > (We have actually seen similar problems before with people trying > to dump and reload database containing contrib modules. pg_migrator > is not creating a problem that wasn't there before, it's just making > it worse.) > > Comments, better ideas? maybe, get_raw_page_v2, etc? I suppose you could run into situation with multiple versions of the function w/same # of arguments? merlin
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Fujii MasaoДата:
Сообщение: Re: postmaster recovery and automatic restart suppression