On Fri, Feb 3, 2017 at 10:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Thu, Feb 2, 2017 at 11:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I just made the C code agree with what the SQL declarations for the
>>> functions say.
>
>> Doesn't look like it to me. You changed a bunch of places that say
>> UInt32GetDatum to UInt64GetDatum, but the SQL type certainly isn't
>> unsigned.
>
> The machines don't care about that. They do care about the width of
> the datum. Particularly on 32-bit hardware, where one width is
> pass-by-val and the other isn't. (Also, if your point is that you
> wish we had a uint64 SQL type, I doubt we're going there.)
I know the machines don't care about that, but I still think it'd be a
better idea to be consistent with the SQL types throughout rather than
only in places where failing to do so actually breaks something. It's
not much of an abstraction layer if we're only kinda-sorta rigorous
about using it correctly. If nothing else, it obscures best practice
for new patch authors.
> What needs to be resolved to decide if any of this is actually sane is to
> figure out which of these values need to be int64 on the SQL side because
> (a) they could practically exceed the range of signed int32 and (b) it
> would bother us to show such values as negative rather than large
> positive. I suspect that not all the things currently declared as int64
> really need to be. I also remain unhappy that we can't manage to be
> consistent about what a BlockNumber parameter is represented as.
The existing usage is mixed. For example, gin_metapage_info() returns
pending_head and pending_tail as bigint, and those are BlockNumber at
the C level. But bt_metap returns fastroot as int4, and that's also a
BlockNumber at the C level. For my money, int8 is a better choice
because I don't like the idea of the block number rolling over to a
negative value for a large relation, but I probably wouldn't bother
breaking compatibility for the existing cases where it's been done
otherwise, because relations of at least 16TB in size are not yet very
common. At some point we may have to bite that bullet but maybe not
today.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company