> 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
