Re: pageinspect: add tuple_data_record()

2018-10-17 Thread James Coleman
> I don't see why you'd get that error, if you re-add a column (with a > different type), as I did above? Obviously the "replacement" column > addition would need to be committed. > Here's my test case: CREATE TABLE t3(i INTEGER); BEGIN; ALTER TABLE t3 ADD COLUMN blarg INT; INSERT INTO t3(bla

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread Andres Freund
On 2018-10-17 17:02:20 -0400, James Coleman wrote: > > There's plenty ways it can go horribly wrong. Let's start with something > > simple: > > > > BEGIN; > > ALTER TABLE ... ADD COLUMN blarg INT; > > INSERT ... (blag) VALUES (132467890); > > ROLLBACK; > > > > ALTER TABLE ... ADD COLUMN blarg TEXT;

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread James Coleman
> There's plenty ways it can go horribly wrong. Let's start with something > simple: > > BEGIN; > ALTER TABLE ... ADD COLUMN blarg INT; > INSERT ... (blag) VALUES (132467890); > ROLLBACK; > > ALTER TABLE ... ADD COLUMN blarg TEXT; > > If you now read the table with your function you'll see a dead r

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread Andres Freund
Hi, On 2018-10-17 13:14:17 -0400, James Coleman wrote: > > It's far from only toast that could be problematic here. > > > > Do you have an example in mind? Wouldn’t the tuples have to be corrupted in > some way to have problems with being interpreted as a datum? Or are you > thinking very old tup

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread James Coleman
> It's far from only toast that could be problematic here. > Do you have an example in mind? Wouldn’t the tuples have to be corrupted in some way to have problems with being interpreted as a datum? Or are you thinking very old tuples with a radically different structure to be causing the problem?

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread Andres Freund
Hi, On 2018-10-17 13:04:33 -0400, James Coleman wrote: > Indeed. But I do think your approach - which means that the binary data is > > actually interpreded as a datum of a specific type, drastically > > increases the risk. > > > > > Agreed. > > As I noted earlier, I don't at all think deTOASTing

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread James Coleman
Indeed. But I do think your approach - which means that the binary data is > actually interpreded as a datum of a specific type, drastically > increases the risk. > > Agreed. As I noted earlier, I don't at all think deTOASTing is a must for this function to be valuable, just as tuple_data_split()

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread Andres Freund
On 2018-10-17 12:36:54 -0400, James Coleman wrote: > > > > > > I did compleatly got the question... The question is it safe to split > > tuple > > record into array of raw bytea? It is quite safe from my point of view. > > We > > use only data that is inside the tuple, and info from pg_catalog that

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread Nikolay Shaplov
В письме от 17 октября 2018 12:36:54 пользователь James Coleman написал: > > I did compleatly got the question... The question is it safe to split > > tuple > > record into array of raw bytea? It is quite safe from my point of view. > > We > > use only data that is inside the tuple, and info from p

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread James Coleman
> > > I did compleatly got the question... The question is it safe to split > tuple > record into array of raw bytea? It is quite safe from my point of view. > We > use only data that is inside the tuple, and info from pg_catalog that > describes the tuple structure. So we are not affected if for e

Re: pageinspect: add tuple_data_record()

2018-10-17 Thread Nikolay Shaplov
В письме от 16 октября 2018 19:17:20 пользователь Andres Freund написал: > 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 > > > cou

Re: pageinspect: add tuple_data_record()

2018-10-16 Thread Andres Freund
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

Re: pageinspect: add tuple_data_record()

2018-10-16 Thread Peter Geoghegan
On Tue, Oct 16, 2018 at 6:39 PM James Coleman wrote: > Summary: > The new function tuple_data_record() parallels the existing > tuple_data_split() function, but instead of returning a bytea array of raw > attribute heap values, it returns a row type of the relation being examined. I've been doi

Re: pageinspect: add tuple_data_record()

2018-10-16 Thread James Coleman
> 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

Re: pageinspect: add tuple_data_record()

2018-10-16 Thread Andres Freund
Hi, On 2018-10-16 21:39:02 -0400, James Coleman wrote: > Background: > In my usage of pageinspect for debugging or other purposes I often find it > frustrating that I don't have a way to easily view a heap page's tuple data > as actual records rather than binary data. After encountering this again