Re: per-column generic option
От | Shigeru Hanada |
---|---|
Тема | Re: per-column generic option |
Дата | |
Msg-id | 4E089817.7070001@gmail.com обсуждение исходный текст |
Ответ на | Re: per-column generic option (Kohei KaiGai <kaigai@kaigai.gr.jp>) |
Ответы |
Re: per-column generic option
(Robert Haas <robertmhaas@gmail.com>)
Re: per-column generic option (Kohei KaiGai <kaigai@kaigai.gr.jp>) Re: per-column generic option (Alvaro Herrera <alvherre@commandprompt.com>) |
Список | pgsql-hackers |
(2011/06/26 18:34), Kohei KaiGai wrote: > I checked your patch. Thanks for the review! Please find attached a revised patch. > The backend portion seems to me OK, but I have several questions/comments. > > * This patch should be rebased. > It conflicts with the latest bin/psql/describe.c and > include/catalog/catversion.h. > IIRC, we should not touch catversion.h in submission stage. (It might > be a task of > committer when a patch get upstreamed.) I've rebased against current HEAD, and reverted catversion.h. > * It might be an option to extend attreloptions, instead of the new > attfdwoptions. > Although I didn't track the discussion when pg_foreign_table catalog > that provides > relation level fdw-options, was it impossible or unreasonable to extend existing > design of reloptions/attoptions? > Right now, it accepts only hard-wired options listed at reloptions.c. > But, it seems > to me worthwhile, if it could accept options validated by loadable modules. IIRC someone has objected against storing FDW options in reloptions/attoptions, but I couldn't find such post. I'll follow the discussion again. IMHO, though at present I don't have clear proof, separating FDW options from access method options seems better than merging them, but I should learn more about AM mechanism to clarify this issue. Please check other issues first. > * pg_dump shall die when we run it for older postgresql version. > > This patch does not modify queries to older postgresql version at > getTableAttrs(). > In the result, this index shall be set by -1. > + i_attfdwoptions = PQfnumber(res, "attfdwoptions"); > > Then, PGgetvalue() returns NULL for unranged column number, and strdup() > shall cause segmentation fault. > + tbinfo->attfdwoptions[j] = strdup(PQgetvalue(res, j, > i_attfdwoptions)); > > In fact, I tried to run the patched pg_dump towards v9.0.2 > [kaigai@vmlinux pg_dump]$ ./pg_dump postgres > pg_dump: column number -1 is out of range 0..14 > Segmentation fault > > My recommendation is to append "NULL as attfdwoptions" on the queries to > older versions. It eventually makes PGgetvalue() to return an empty string, > then strdup() does not cause a problem. Fixed in the way you've recommended, and tested against 8.4. I should have noticed that same technique is used in some other places... BTW, I also have found an unnecessary FIXME comment and removed it. Please see the line 2845 of src/backend/catalog/heap.c (InsertPgAttributeTuple) for the correction. Regards, -- Shigeru Hanada
Вложения
В списке pgsql-hackers по дате отправления: