From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | James Coleman <jtc331(at)gmail(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, Nikolay Shaplov <dhyan(at)nataraj(dot)su> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pageinspect: add tuple_data_record() |
Date: | 2018-10-17 02:17:20 |
Message-ID: | 20181017021720.bfdpziwegxd2abc3@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2018-10-16 22:05:28 -0400, James Coleman wrote:
> > I don't think it's entirely safe to do so for invisible rows. The toast
> > references could be reused, constraints be violated, etc. So while I
> > could have used this several times before, it's also very likely a good
> > way to cause trouble. It'd probably be ok to just fetch the binary
> > value of the columns, but detoasting sure ain't ok.
> >
>
> Wouldn’t that be equally true for the already existing toast option to
> tuple_data_split()?
Yes. Whoever added that didn't think far enough. Teodor, Nikolay?
> Is “unsafe” here something that would lead to a crash? Or just returning
> invalid data? Given that pageinspect is already clearly an internal viewing
> of data, I think allowing invalid or limited data is a reasonable result.
Both.
> Alternatively, would returning only inline values be safe (and say,
> returning null for any toasted data)?
I think that'd be somewhat confusing.
I don't think it's necessarily just toast that's a problem here, that
was just an obvious point.
> Yes, though seems like maybe the style guide could be a bit more
> descriptive in some of these areas to be more friendly to newcomers. In
> contrast the wiki page for submitting a patch is extremely detailed.
I think there's a lot in that area we should improve.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Yang Jie | 2018-10-17 03:18:36 | Implementation of Flashback Query |
Previous Message | Peter Geoghegan | 2018-10-17 02:05:53 | Re: pageinspect: add tuple_data_record() |