On Tue, Nov 17, 2015 at 7:54 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > I think this might do the wrong thing with block numbers above 0x80000000 > and/or offset numbers above 0x8000. I'd be more comfortable about it if > + encoded = ((int64) block << 16) | offset; > were > + encoded = ((uint64) block << 16) | (uint16) offset; > so that there's no risk of the compiler deciding to sign-extend rather > than zero-fill either input.
I don't have a problem with your alternative, but I don't see any risk with the original. It is recommended by various coding standards to only use bitwise operators on unsigned operands, so that's a good enough reason, I suppose. > Also, I think it'd be a good idea to explicitly set indexcursor = NULL > in the tuplesort_empty case; the previous coding was effectively doing > that. It's true that the code shouldn't attempt to touch the value, > but it's better not to have dangling pointers lying around. The code started that way, but I removed the "indexcursor = NULL" because the previous coding was *not* doing that. tuplesort_getdatum() was not setting the passed Datum pointer (which points to indexcursor here) in the event of returning false. Maybe I should have left in the code setting indexcursor = NULL all the same. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
