Re: IndexTupleDSize macro seems redundant

2018-02-28 Thread Tom Lane
Stephen Frost writes: > Updated (combined) patch attached for review. I went through and looked > again to make sure there weren't any cases of making an unaligned > pointer to a struct and didn't see any, and I added some comments to > _bt_restore_page(). This seems to have

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Robert Haas
On Thu, Jan 11, 2018 at 9:17 PM, Stephen Frost wrote: > Great, thanks, I'll mark it as Ready For Committer then. > > Robert, since you were on this thread and the patch is mostly yours > anyway, did you want to commit it? I'm happy to do so also, either way. Feel free. --

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Stephen Frost
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > Updated (combined) patch attached for review. I went through and looked > > again to make sure there weren't any cases of making an unaligned > > pointer to a struct and didn't see any, and I added some

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Tom Lane
Stephen Frost writes: > Updated (combined) patch attached for review. I went through and looked > again to make sure there weren't any cases of making an unaligned > pointer to a struct and didn't see any, and I added some comments to > _bt_restore_page(). Looks OK from

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Stephen Frost
Greetings Tom, Robert, Ildar, all, * Stephen Frost (sfr...@snowman.net) wrote: > That said, since it's not aligned, regardless of the what craziness the > compiler might try to pull, we probably shouldn't go casting it > to something that later hackers might think will be aligned, but we > should

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Robert Haas
On Thu, Jan 11, 2018 at 1:26 PM, Tom Lane wrote: >> I certainly hadn't been thinking about that. I didn't see any >> issues in my testing (where I created a table with a btree index and >> insert'd a bunch of records into and then killed the server, forcing WAL >> replay and

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Andres Freund
On 2018-01-11 13:26:27 -0500, Tom Lane wrote: > I wonder whether there is a way to get alignment traps on Intel-type > hardware. It's getting less and less likely that most hackers are > developing on anything else, so that we don't see gotchas of this > type until code hits the buildfarm (and

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Tom Lane
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> I'm on board with Stephen's changes, except in _bt_restore_page. >> The issue there is that the "from" pointer isn't necessarily adequately >> aligned to be considered an IndexTuple pointer; that's why we're

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Stephen Frost
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > I'll leave the patch status in 'Needs review' since there's more > > changes, but hopefully someone can take a look and we can move this > > along, seems like a pretty small and reasonable improvement. >

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Tom Lane
Stephen Frost writes: > I'll leave the patch status in 'Needs review' since there's more > changes, but hopefully someone can take a look and we can move this > along, seems like a pretty small and reasonable improvement. I'm on board with Stephen's changes, except in

Re: IndexTupleDSize macro seems redundant

2018-01-11 Thread Stephen Frost
Robert, all, * Robert Haas (robertmh...@gmail.com) wrote: > On Tue, Nov 21, 2017 at 9:26 AM, Amit Kapila wrote: > > +1. I was also once confused with these macros. I think this is a > > good cleanup. On a quick look, I don't see any problem with your > > changes. > >

Re: IndexTupleDSize macro seems redundant

2017-11-30 Thread Peter Geoghegan
On Thu, Nov 30, 2017 at 1:48 PM, Robert Haas wrote: > One difference between those two macros is that IndexTupleSize > forcibly casts the argument to IndexTuple, which means that you don't > get any type-checking when you use that one. I suggest that in > addition to

Re: IndexTupleDSize macro seems redundant

2017-11-21 Thread Amit Kapila
On Mon, Nov 20, 2017 at 9:01 PM, Ildar Musin wrote: > Hi all, > > While I was looking through the indexes code I got confused by couple of > macros - IndexTupleSize() and IndexTupleDSize() - which seem to do the same > thing with only difference that the first one takes