From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Should TIDs be typbyval = FLOAT8PASSBYVAL to speed up CREATE INDEX CONCURRENTLY? |
Date: | 2015-11-17 18:59:05 |
Message-ID: | CAM3SWZTek0wKQRDteKPZpExpjR7YW6SGyNWtgE7XVemJo1W6wQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Nov 17, 2015 at 7:54 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2015-11-17 19:02:48 | Re: [PATCH] Skip ALTER x SET SCHEMA if the schema didn't change |
Previous Message | Alvaro Herrera | 2015-11-17 18:50:36 | Re: Question concerning XTM (eXtensible Transaction Manager API) |