On Mon, Jan 18, 2021 at 3:52 AM Heikki Linnakangas <hlinn...@iki.fi> wrote:
> On 18/01/2021 00:35, Peter Geoghegan wrote: > > On Sun, Jan 17, 2021 at 12:50 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > >> I noticed that gist_page_items() thinks it can hold inter_call_data->rel > >> open across a series of calls. That's completely unsafe: the executor > >> might not run the call series to completion (see LIMIT), resulting in > >> relcache leak complaints. > > Fixed, thanks! I changed it to return a tuplestore. > > > It also has the potential to run into big problems should the user > > input a raw page image with an regclass-argument-incompatible tuple > > descriptor. Maybe that's okay (this is a tool for experts), but it > > certainly is a consideration. > > I'm not sure I understand. It's true that the raw page image can contain > data from a different index, or any garbage really. And the function > will behave badly if you do that. That's an accepted risk with > pageinspect functions, that's why they're superuser-only, although some > of them are more tolerant of corrupt pages than others. The > gist_page_items_bytea() variant doesn't try to parse the key data and is > less likely to crash on bad input. > > - Heikki > > > The patch (0001-Add-bool-column-for-LP_DEAF-flag-to-GiST-pageinspect.patch ) does not apply successfully and has multiple hanks failed. http://cfbot.cputube.org/patch_32_2824.log patching file contrib/pageinspect/gistfuncs.c Hunk #1 FAILED at 151. Hunk #2 FAILED at 175. Hunk #3 FAILED at 245. Hunk #4 FAILED at 271. ... Can we get a rebase? I am marking the patch "Waiting on Author" -- Ibrar Ahmed
