From: | James Coleman <jtc331(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pageinspect: add tuple_data_record() |
Date: | 2018-10-17 02:05:28 |
Message-ID: | CAAaqYe-cmYWBtsuo==N5kHDw09r4h3=3tk=_vhYEE_pgmcm=PQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2018-10-17 02:05:53 | Re: pageinspect: add tuple_data_record() |
Previous Message | Andres Freund | 2018-10-17 01:46:59 | Re: pageinspect: add tuple_data_record() |