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()?
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.
Alternatively, would returning only inline values be safe (and say, returning null for any toasted data)?
> This is my first patch submission, so I encountered a few questions that
> the coding style docs didn't seem to address, like whether it's preferable
> to line up variable declarations by name.
There's ./src/tools/pgindent/pgindent (which needs an external dep), to
do that kind of thing...
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.
Thanks,
James Coleman