Re: pageinspect: add tuple_data_record() - Mailing list pgsql-hackers

From James Coleman
Subject Re: pageinspect: add tuple_data_record()
Date
Msg-id CAAaqYe-cmYWBtsuo==N5kHDw09r4h3=3tk=_vhYEE_pgmcm=PQ@mail.gmail.com
Whole thread Raw
In response to Re: pageinspect: add tuple_data_record()  (Andres Freund <andres@anarazel.de>)
Responses Re: pageinspect: add tuple_data_record()  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers

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

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: pageinspect: add tuple_data_record()
Next
From: Peter Geoghegan
Date:
Subject: Re: pageinspect: add tuple_data_record()