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
