Обсуждение: Making type Datum be 8 bytes everywhere
In a discussion on Discord (in the PG #core-hacking channel,
which unfortunately is inaccessible to non-members), Andres
and Robert complained about the development/maintenance costs
of continuing to support 32-bit platforms. Here is a modest
proposal to reduce those costs without going so far as to
entirely desupport such platforms: let's require them to use
8-byte Datums even though that's probably not a native data
type for them. That lets us get rid of logic to support the
!USE_FLOAT8_BYVAL case, and allows a few other simplifications.
The attached patch switches to 8-byte Datums everywhere, but
doesn't make any effort to remove the now-dead code. I made
it just as a proof-of-concept that this can work. It compiled
cleanly and passed check-world for me on a 32-bit FreeBSD
image.
I've not looked into the performance consequences. We probably
should at least try to measure that, though I'm not sure what
our threshold of pain would be for deciding not to do this.
Thoughts?
regards, tom lane
From 175803f26dd76be274c469f958aef9c391bf88ff Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 17 Jul 2025 19:55:31 -0400
Subject: [PATCH v0] Make type Datum be 8 bytes wide everywhere.
This very WIP patch aims to make sizeof(Datum) be 8 on all
platforms including 32-bit ones. The objective is to allow
USE_FLOAT8_BYVAL to be true everywhere, and in consequence to
remove a lot of code that is specific to pass-by-reference
handling of float8, int8, etc. In this way we can reduce the
maintenance effort involved in supporting 32-bit platforms,
without going so far as to actually desupport them. Since Datum
is strictly an in-memory concept, this has no impact on on-disk
storage, though an initdb or pg_upgrade will be needed to fix
affected pg_type.typbyval entries.
We can expect that this change will make 32-bit builds a bit slower
and more memory-hungry, although being able to use pass-by-value
handling of 8-byte types may buy back some of that. But we stopped
optimizing for 32-bit cases a long time ago, and this seems like
just another step on that path.
This initial patch simply forces the correct type definition and
USE_FLOAT8_BYVAL setting, and cleans up a couple of minor compiler
complaints that ensued. This is sufficient for testing purposes.
It does compile cleanly and pass check-world for me on a 32-bit
platform.
If we pursue this path, we would of course want to remove all
the now-dead code (search for mentions of SIZEOF_DATUM and
USE_FLOAT8_BYVAL to find likely places to clean up). I have
not bothered with that yet. A more interesting question is
whether there are comments or calculations that need adjustment.
There is probably an effect on the largest array that we can
support in 32-bit mode, for example.
---
src/backend/storage/ipc/ipc.c | 2 +-
src/include/nodes/nodes.h | 8 ++++++--
src/include/pg_config_manual.h | 13 ++++---------
src/include/postgres.h | 13 +++++++------
4 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index 567739b5be9..2704e80b3a7 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -399,7 +399,7 @@ cancel_before_shmem_exit(pg_on_exit_callback function, Datum arg)
before_shmem_exit_list[before_shmem_exit_index - 1].arg == arg)
--before_shmem_exit_index;
else
- elog(ERROR, "before_shmem_exit callback (%p,0x%" PRIxPTR ") is not the latest entry",
+ elog(ERROR, "before_shmem_exit callback (%p,0x%" PRIx64 ") is not the latest entry",
function, arg);
}
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index fbe333d88fa..b2dc380b57b 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -188,6 +188,8 @@ castNodeImpl(NodeTag type, void *ptr)
* ----------------------------------------------------------------
*/
+#ifndef FRONTEND
+
/*
* nodes/{outfuncs.c,print.c}
*/
@@ -198,7 +200,7 @@ extern void outNode(struct StringInfoData *str, const void *obj);
extern void outToken(struct StringInfoData *str, const char *s);
extern void outBitmapset(struct StringInfoData *str,
const struct Bitmapset *bms);
-extern void outDatum(struct StringInfoData *str, uintptr_t value,
+extern void outDatum(struct StringInfoData *str, Datum value,
int typlen, bool typbyval);
extern char *nodeToString(const void *obj);
extern char *nodeToStringWithLocations(const void *obj);
@@ -212,7 +214,7 @@ extern void *stringToNode(const char *str);
extern void *stringToNodeWithLocations(const char *str);
#endif
extern struct Bitmapset *readBitmapset(void);
-extern uintptr_t readDatum(bool typbyval);
+extern Datum readDatum(bool typbyval);
extern bool *readBoolCols(int numCols);
extern int *readIntCols(int numCols);
extern Oid *readOidCols(int numCols);
@@ -235,6 +237,8 @@ extern void *copyObjectImpl(const void *from);
*/
extern bool equal(const void *a, const void *b);
+#endif /* !FRONTEND */
+
/*
* Typedef for parse location. This is just an int, but this way
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 125d3eb5fff..7e1aa422332 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -74,17 +74,12 @@
#define PARTITION_MAX_KEYS 32
/*
- * Decide whether built-in 8-byte types, including float8, int8, and
- * timestamp, are passed by value. This is on by default if sizeof(Datum) >=
- * 8 (that is, on 64-bit platforms). If sizeof(Datum) < 8 (32-bit platforms),
- * this must be off. We keep this here as an option so that it is easy to
- * test the pass-by-reference code paths on 64-bit platforms.
- *
- * Changing this requires an initdb.
+ * This symbol is now vestigial: built-in 8-byte types, including float8,
+ * int8, and timestamp, are always passed by value since we require Datum
+ * to be wide enough to permit that. We continue to define the symbol here
+ * so as not to unnecessarily break extension code.
*/
-#if SIZEOF_VOID_P >= 8
#define USE_FLOAT8_BYVAL 1
-#endif
/*
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 8a41a668687..f2d3ee5af3b 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -58,15 +58,18 @@
/*
* A Datum contains either a value of a pass-by-value type or a pointer to a
- * value of a pass-by-reference type. Therefore, we require:
- *
- * sizeof(Datum) == sizeof(void *) == 4 or 8
+ * value of a pass-by-reference type. Therefore, we must have
+ * sizeof(Datum) >= sizeof(void *). No current or foreseeable Postgres
+ * platform has pointers wider than 8 bytes, and standardizing on Datum being
+ * exactly 8 bytes has advantages in reducing cross-platform differences.
*
* The functions below and the analogous functions for other types should be used to
* convert between a Datum and the appropriate C type.
*/
-typedef uintptr_t Datum;
+typedef uint64_t Datum;
+
+#define SIZEOF_DATUM 8
/*
* A NullableDatum is used in places where both a Datum and its nullness needs
@@ -83,8 +86,6 @@ typedef struct NullableDatum
/* due to alignment padding this could be used for flags for free */
} NullableDatum;
-#define SIZEOF_DATUM SIZEOF_VOID_P
-
/*
* DatumGetBool
* Returns boolean value of a datum.
--
2.43.5
Hi,
On 2025-07-17 20:09:57 -0400, Tom Lane wrote:
> In a discussion on Discord (in the PG #core-hacking channel,
> which unfortunately is inaccessible to non-members), Andres
> and Robert complained about the development/maintenance costs
> of continuing to support 32-bit platforms. Here is a modest
> proposal to reduce those costs without going so far as to
> entirely desupport such platforms: let's require them to use
> 8-byte Datums even though that's probably not a native data
> type for them. That lets us get rid of logic to support the
> !USE_FLOAT8_BYVAL case, and allows a few other simplifications.
>
> The attached patch switches to 8-byte Datums everywhere, but
> doesn't make any effort to remove the now-dead code.
Thanks for writing that!
> I made it just as a proof-of-concept that this can work. It compiled
> cleanly and passed check-world for me on a 32-bit FreeBSD image.
Interestingly it generates a *lot* of warnings here when building for 32 bit
with gcc. One class of complaints is about DatumGetPointer() and
PointerGetDatum() casting between different sizes:
../../../../../home/andres/src/postgresql/src/include/postgres.h: In function 'DatumGetPointer':
../../../../../home/andres/src/postgresql/src/include/postgres.h:320:16: warning: cast to pointer from integer of
differentsize [-Wint-to-pointer-cast]
320 | return (Pointer) X;
| ^
../../../../../home/andres/src/postgresql/src/include/postgres.h: In function 'PointerGetDatum':
../../../../../home/andres/src/postgresql/src/include/postgres.h:330:16: warning: cast from pointer to integer of
differentsize [-Wpointer-to-int-cast]
330 | return (Datum) X;
| ^
And then there's a set of complains due to code converting from NULL to Datum
without going through PointerGetDatum():
../../../../../home/andres/src/postgresql/src/include/access/htup_details.h: In function 'fastgetattr':
../../../../../home/andres/src/postgresql/src/include/access/htup_details.h:887:32: warning: cast from pointer to
integerof different size [-Wpointer-to-int-cast]
887 | return (Datum) NULL;
../../../../../home/andres/src/postgresql/src/include/access/itup.h: In function 'index_getattr':
../../../../../home/andres/src/postgresql/src/include/access/itup.h:157:32: warning: cast from pointer to integer of
differentsize [-Wpointer-to-int-cast]
157 | return (Datum) NULL;
A third, not quite as verbose set is random code in .c files missing uses of
DatumGetPointer(). There are lot of those.
A fourth class is passing a Datum to VAR* macros. They're a bit too verbose to
paste here, but it's just a variation of the above. I'm not really sure what
our intended use of them is, do we intend to pass pointers or datums to the
macros? I suspect we'd have to move the casts into the varlena macros,
otherwise we'll have to add DatumGetPointer() uses all over.
One of these days I should again try the experiment of making Datum into a
struct, to automatically catch omissions of datum <-> native type. Having them
be silent most of the time really sucks. I suspect that if we get the
64bit-datum-on-32bit-platform code to be warning-free, it'd get a lot easier
to struct-ify Datum. I don't recall the details, but I suspect that all the
varlena macros etc were the problem with that.
> I've not looked into the performance consequences. We probably
> should at least try to measure that, though I'm not sure what
> our threshold of pain would be for deciding not to do this.
From my POV the threshold would have to be rather high for backend code. Less
so in libpq, but that's not affected here.
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2025-07-17 20:09:57 -0400, Tom Lane wrote:
>> I made it just as a proof-of-concept that this can work. It compiled
>> cleanly and passed check-world for me on a 32-bit FreeBSD image.
> Interestingly it generates a *lot* of warnings here when building for 32 bit
> with gcc.
Oh, that's annoying. I tested it with
$ cc --version
FreeBSD clang version 13.0.0 (git@github.com:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a303)
Target: i386-unknown-freebsd13.1
Thread model: posix
InstalledDir: /usr/bin
which is slightly back-rev but not that old. Which gcc did you use?
> One class of complaints is about DatumGetPointer() and
> PointerGetDatum() casting between different sizes:
> ../../../../../home/andres/src/postgresql/src/include/postgres.h: In function 'DatumGetPointer':
> ../../../../../home/andres/src/postgresql/src/include/postgres.h:320:16: warning: cast to pointer from integer of
differentsize [-Wint-to-pointer-cast]
> 320 | return (Pointer) X;
> | ^
> ../../../../../home/andres/src/postgresql/src/include/postgres.h: In function 'PointerGetDatum':
> ../../../../../home/andres/src/postgresql/src/include/postgres.h:330:16: warning: cast from pointer to integer of
differentsize [-Wpointer-to-int-cast]
> 330 | return (Datum) X;
> | ^
We might be able to silence those with intermediate casts to uintptr_t,
perhaps?
> And then there's a set of complains due to code converting from NULL to Datum
> without going through PointerGetDatum():
> 887 | return (Datum) NULL;
This code just strikes me as tin-eared. Our normal locution for
that is "(Datum) 0", and I see no reason to deviate.
> A third, not quite as verbose set is random code in .c files missing uses of
> DatumGetPointer(). There are lot of those.
Yeah, that's stuff we ought to fix anyway. Same with VAR* macros.
> One of these days I should again try the experiment of making Datum into a
> struct, to automatically catch omissions of datum <-> native type. Having them
> be silent most of the time really sucks.
Perhaps. I'd be a little sad if the "(Datum) 0" notation stops
working, because there are sure a lot of those.
>> I've not looked into the performance consequences. We probably
>> should at least try to measure that, though I'm not sure what
>> our threshold of pain would be for deciding not to do this.
> From my POV the threshold would have to be rather high for backend code. Less
> so in libpq, but that's not affected here.
I don't know if it's "rather high" or not, but that seems like
the gating factor that ought to be checked before putting in
more work.
regards, tom lane
Hi, On 2025-07-18 13:24:32 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2025-07-17 20:09:57 -0400, Tom Lane wrote: > >> I made it just as a proof-of-concept that this can work. It compiled > >> cleanly and passed check-world for me on a 32-bit FreeBSD image. > > > Interestingly it generates a *lot* of warnings here when building for 32 bit > > with gcc. > > Oh, that's annoying. I tested it with > > $ cc --version > FreeBSD clang version 13.0.0 (git@github.com:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a303) > Target: i386-unknown-freebsd13.1 > Thread model: posix > InstalledDir: /usr/bin > > which is slightly back-rev but not that old. Which gcc did you use? That was gcc 14. > > One class of complaints is about DatumGetPointer() and > > PointerGetDatum() casting between different sizes: > > > ../../../../../home/andres/src/postgresql/src/include/postgres.h: In function 'DatumGetPointer': > > ../../../../../home/andres/src/postgresql/src/include/postgres.h:320:16: warning: cast to pointer from integer of differentsize [-Wint-to-pointer-cast] > > 320 | return (Pointer) X; > > | ^ > > ../../../../../home/andres/src/postgresql/src/include/postgres.h: In function 'PointerGetDatum': > > ../../../../../home/andres/src/postgresql/src/include/postgres.h:330:16: warning: cast from pointer to integer of differentsize [-Wpointer-to-int-cast] > > 330 | return (Datum) X; > > | ^ > > We might be able to silence those with intermediate casts to uintptr_t, > perhaps? Yep, that does the trick. > >> I've not looked into the performance consequences. We probably > >> should at least try to measure that, though I'm not sure what > >> our threshold of pain would be for deciding not to do this. > > > From my POV the threshold would have to be rather high for backend code. Less > > so in libpq, but that's not affected here. > > I don't know if it's "rather high" or not, but that seems like > the gating factor that ought to be checked before putting in > more work. The hard bit would be to determine what workload to measure. Something like pgbench probably won't suffer meaningfully, there's just not enough passing of values around. For a bit I thought it'd need to be a workload that does a lot of int4 math or such, but I doubt the overhead of it matters sufficiently there. Then I realized that the biggest issue probably would be a query that does a lot of tuple deforming of 4 byte values, while not actually accessing them? Can you think of a worse workload than that? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2025-07-18 13:24:32 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> One class of complaints is about DatumGetPointer() and
>>> PointerGetDatum() casting between different sizes:
>> We might be able to silence those with intermediate casts to uintptr_t,
>> perhaps?
> Yep, that does the trick.
Cool, thanks for checking.
>>> I've not looked into the performance consequences. We probably
>>> should at least try to measure that, though I'm not sure what
>>> our threshold of pain would be for deciding not to do this.
> The hard bit would be to determine what workload to measure. Something like
> pgbench probably won't suffer meaningfully, there's just not enough passing of
> values around.
I'm disinclined to put in a huge amount of effort looking for the
worst case. We established long ago that we weren't going to
optimize for 32-bit anymore. So as long as this doesn't completely
tank performance on 32-bit, I'm satisfied. I'd almost say that
if standard pgbench doesn't notice the change, that's good enough.
regards, tom lane
Hi,
Thank you for working on this!
On Wed, 23 Jul 2025 at 22:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I'm disinclined to put in a huge amount of effort looking for the
> worst case. We established long ago that we weren't going to
> optimize for 32-bit anymore. So as long as this doesn't completely
> tank performance on 32-bit, I'm satisfied. I'd almost say that
> if standard pgbench doesn't notice the change, that's good enough.
I did a basic pgbench benchmark on a 32-bit build and there is no change.
$ pgbench -i -s 100 test
$ pgbench -c 16 -j 16 -b $type -T 150 test
TPS results are:
select-only:
master: 215654
patched: 215751
simple-update:
master: 4454
patched: 4446
tpcb-like:
master: 4094
patched: 4128
--
Regards,
Nazir Bilal Yavuz
Microsoft
Nazir Bilal Yavuz <byavuz81@gmail.com> writes:
> On Wed, 23 Jul 2025 at 22:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm disinclined to put in a huge amount of effort looking for the
>> worst case. We established long ago that we weren't going to
>> optimize for 32-bit anymore. So as long as this doesn't completely
>> tank performance on 32-bit, I'm satisfied. I'd almost say that
>> if standard pgbench doesn't notice the change, that's good enough.
> I did a basic pgbench benchmark on a 32-bit build and there is no change.
Thanks for doing that! For me, that's enough evidence to move
forward.
regards, tom lane
[ getting back to looking at this ]
Andres Freund <andres@anarazel.de> writes:
> On 2025-07-17 20:09:57 -0400, Tom Lane wrote:
>> The attached patch switches to 8-byte Datums everywhere, but
>> doesn't make any effort to remove the now-dead code.
> Thanks for writing that!
> Interestingly it generates a *lot* of warnings here when building for 32 bit
> with gcc.
Yeah, I can reproduce that if I use gcc. Interesting that casting
between pointer and different-size integer is a default warning on gcc
but not clang. The major stumbling block to cleaning that up seems
to be what you noted:
> A fourth class is passing a Datum to VAR* macros. They're a bit too verbose to
> paste here, but it's just a variation of the above. I'm not really sure what
> our intended use of them is, do we intend to pass pointers or datums to the
> macros? I suspect we'd have to move the casts into the varlena macros,
> otherwise we'll have to add DatumGetPointer() uses all over.
Right, we have for a long time not worried about whether VARDATA and
the allied macros are being fed a pointer or a Datum. I recall that
somebody tried to make those macros into static inlines awhile back,
and failed because of the lack of clarity about how to declare their
arguments. I think the way forward here is to tackle that head-on
and split the top-level macros into two static inlines, along the
lines of
VARDATA(Pointer ptr)
and
VARDATA_D(Datum dat)
where the _D versions are simply DatumGetPointer and then call the
non-D versions.
I'm giving the traditional names to the Pointer variants because it
turns out that way more places would have to change if we do it the
other way: in a rough count, about 50 versus about 1700. (This is
counting only the core backend.) Beyond that, though, bikeshedding
on the naming is welcome.
> One of these days I should again try the experiment of making Datum into a
> struct, to automatically catch omissions of datum <-> native type.
It looks like the main remaining thing we'd need to change for that
to be possible is to replace "(Datum) 0" with some macro, say
"INVALID_DATUM". I don't especially want to do that though: there
are upwards of 600 occurrences in our tree, so the consequences for
back-patch hazards seem to me to outweigh the benefit.
regards, tom lane
On 18.07.25 18:26, Andres Freund wrote: > One of these days I should again try the experiment of making Datum into a > struct, to automatically catch omissions of datum <-> native type. Having them > be silent most of the time really sucks. I suspect that if we get the > 64bit-datum-on-32bit-platform code to be warning-free, it'd get a lot easier > to struct-ify Datum. I don't recall the details, but I suspect that all the > varlena macros etc were the problem with that. Patch posted here for demonstration: https://www.postgresql.org/message-id/8246d7ff-f4b7-4363-913e-827dadfeb145%40eisentraut.org
On 30.07.25 18:06, Tom Lane wrote: > Right, we have for a long time not worried about whether VARDATA and > the allied macros are being fed a pointer or a Datum. I recall that > somebody tried to make those macros into static inlines awhile back, > and failed because of the lack of clarity about how to declare their > arguments. I don't know if that was me, but I have posted a patch about this now: https://www.postgresql.org/message-id/928ea48f-77c6-417b-897c-621ef16685a6%40eisentraut.org > I think the way forward here is to tackle that head-on > and split the top-level macros into two static inlines, along the > lines of > VARDATA(Pointer ptr) > and > VARDATA_D(Datum dat) > where the _D versions are simply DatumGetPointer and then call the > non-D versions. > > I'm giving the traditional names to the Pointer variants because it > turns out that way more places would have to change if we do it the > other way: in a rough count, about 50 versus about 1700. (This is > counting only the core backend.) Beyond that, though, bikeshedding > on the naming is welcome. In my patch, I just added the missing DatumGetPointer() calls, which seemed easy enough. There is precedent for having two different functions, though, like att_addlength_pointer() and att_addlength_datum().
Peter Eisentraut <peter@eisentraut.org> writes:
> On 30.07.25 18:06, Tom Lane wrote:
>> I'm giving the traditional names to the Pointer variants because it
>> turns out that way more places would have to change if we do it the
>> other way: in a rough count, about 50 versus about 1700. (This is
>> counting only the core backend.) Beyond that, though, bikeshedding
>> on the naming is welcome.
> In my patch, I just added the missing DatumGetPointer() calls, which
> seemed easy enough.
I had an earlier patch version that also did that, but it seemed
kind of verbose to me: adding "_D" is much shorter than adding
"DatumGetPointer()", and fewer parens seems good for readability.
One interesting thing I noted is that in some modules we already
were applying DatumGetPointer where needed (mostly, at least).
The patch I just posted in your other thread also simplifies those
cases to use the "_D" notation, which makes it longer than strictly
necessary. But I think consistency of notation is good.
> There is precedent for having two different functions, though, like
> att_addlength_pointer() and att_addlength_datum().
Yeah ... those two macros could stand to be cleaned up too, per
the notes in their comments. But I don't think we need to fix
that today.
regards, tom lane
On 18.07.25 02:09, Tom Lane wrote: > The attached patch switches to 8-byte Datums everywhere, but > doesn't make any effort to remove the now-dead code. Is the plan to support only exactly Datums of size 8, or Datums of size at least 8? There are some optimistic conditionals like #if SIZEOF_DATUM >= 8 I don't expect it to work right now with larger sizes, but maybe it should? (uuid pass-by-value!?!)
Peter Eisentraut <peter@eisentraut.org> writes:
> On 18.07.25 02:09, Tom Lane wrote:
>> The attached patch switches to 8-byte Datums everywhere, but
>> doesn't make any effort to remove the now-dead code.
> Is the plan to support only exactly Datums of size 8, or Datums of size
> at least 8?
My plan was to hard-wire it at 8 permanently. It's pretty hard to
believe that there will be hardware on which sizeof(Datum) == 16
would be a reasonable choice anytime while people are still using C.
You'd want 16-byte registers and native operations, and I don't
see any manufacturers headed in that direction.
It could be reasonable to keep provisions for that as long as we still
have active hardware and testing for two different sizes of Datum.
However, once we kill off testing of sizeof(Datum) == 4, I think the
code will acquire hard assumptions that sizeof(Datum) == 8 pretty
soon. If I were upset with that prospect I would not be proposing
this change.
regards, tom lane
On Thu, Jul 31, 2025 at 10:27:37AM -0400, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> In my patch, I just added the missing DatumGetPointer() calls, which >> seemed easy enough. > > I had an earlier patch version that also did that, but it seemed > kind of verbose to me: adding "_D" is much shorter than adding > "DatumGetPointer()", and fewer parens seems good for readability. I have mixed feelings about that, but as long as one is easily able to detect that they should not pass a Datum. At the end, I'm kind of OK-ish with the addition of the _D flavors to have a shortcut for the VARDATA/DatumGetPointer() patterns, as an option. So I'd put +0.5. -- Michael
Вложения
I have just realized that this proposal has a rather nasty defect.
Per the following comment in spgist_private.h:
* If the prefix datum is of a pass-by-value type, it is stored in its
* Datum representation, that is its on-disk representation is of length
* sizeof(Datum). This is a fairly unfortunate choice, because in no other
* place does Postgres use Datum as an on-disk representation; it creates
* an unnecessary incompatibility between 32-bit and 64-bit builds. But the
* compatibility loss is mostly theoretical since MAXIMUM_ALIGNOF typically
* differs between such builds, too. Anyway we're stuck with it now.
This means we cannot change sizeof(Datum), nor reconsider the
pass-by-value classification of any datatype, without potentially
breaking pg_upgrade of some SP-GiST indexes on 32-bit machines.
Now, it looks like this doesn't affect any in-core SP-GiST opclasses.
The only one using a potentially affected type is kd_point_ops which
uses a float8 prefix. That'll have been stored in regular on-disk
format on a 32-bit machine, but if we redefine it as being stored
in 64-bit-Datum format, nothing actually changes. The case that
would be problematic is a prefix type that's 4 bytes or less, and
I don't see any.
A quick search of Debian Code Search doesn't find any extensions
that look like they are using small pass-by-value prefixes either.
So maybe we can get away with just changing this, but it's worrisome.
On the positive side, even if there are any SP-GiST opclasses that
are at risk, the population of installations using them on 32-bit
installs has got to be pretty tiny. And the worst-case answer is
that you'd have to reindex such indexes after pg_upgrade.
BTW, I don't think we can teach pg_upgrade to check for this
hazard, because the SP-GiST APIs are such that the data type
used for prefixes isn't visible at the SQL level.
Do we think that making this change is valuable enough to justify
taking such a risk?
regards, tom lane
On 8/8/25 21:14, Tom Lane wrote: > I have just realized that this proposal has a rather nasty defect. > Per the following comment in spgist_private.h: > > * If the prefix datum is of a pass-by-value type, it is stored in its > * Datum representation, that is its on-disk representation is of length > * sizeof(Datum). This is a fairly unfortunate choice, because in no other > * place does Postgres use Datum as an on-disk representation; it creates > * an unnecessary incompatibility between 32-bit and 64-bit builds. But the > * compatibility loss is mostly theoretical since MAXIMUM_ALIGNOF typically > * differs between such builds, too. Anyway we're stuck with it now. > > This means we cannot change sizeof(Datum), nor reconsider the > pass-by-value classification of any datatype, without potentially > breaking pg_upgrade of some SP-GiST indexes on 32-bit machines. > > Now, it looks like this doesn't affect any in-core SP-GiST opclasses. > The only one using a potentially affected type is kd_point_ops which > uses a float8 prefix. That'll have been stored in regular on-disk > format on a 32-bit machine, but if we redefine it as being stored > in 64-bit-Datum format, nothing actually changes. The case that > would be problematic is a prefix type that's 4 bytes or less, and > I don't see any. > > A quick search of Debian Code Search doesn't find any extensions > that look like they are using small pass-by-value prefixes either. > So maybe we can get away with just changing this, but it's worrisome. > > On the positive side, even if there are any SP-GiST opclasses that > are at risk, the population of installations using them on 32-bit > installs has got to be pretty tiny. I bet it is indistinguishable from zero... > And the worst-case answer is that you'd have to reindex such indexes > after pg_upgrade. ...and this seems like a reasonable answer if anyone pops up. > BTW, I don't think we can teach pg_upgrade to check for this > hazard, because the SP-GiST APIs are such that the data type > used for prefixes isn't visible at the SQL level. > > Do we think that making this change is valuable enough to justify > taking such a risk? yes +1 -- Joe Conway PostgreSQL Contributors Team Amazon Web Services: https://aws.amazon.com
Joe Conway <mail@joeconway.com> writes:
> On 8/8/25 21:14, Tom Lane wrote:
>> I have just realized that this proposal has a rather nasty defect.
>> ...
>> On the positive side, even if there are any SP-GiST opclasses that
>> are at risk, the population of installations using them on 32-bit
>> installs has got to be pretty tiny.
> I bet it is indistinguishable from zero...
That's my bet too.
>> And the worst-case answer is that you'd have to reindex such indexes
>> after pg_upgrade.
> ...and this seems like a reasonable answer if anyone pops up.
>> Do we think that making this change is valuable enough to justify
>> taking such a risk?
> yes +1
I've now fleshed out the patch series with some cleanup of code that's
been rendered dead. The 0001 patch is nearly the same as before,
but thanks to all the work Peter did, it doesn't trigger a pile of
gcc warnings (at least, not for me).
regards, tom lane
From 91fb37fab96222625e37408cb564d5b293d0851f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 8 Aug 2025 19:15:59 -0400
Subject: [PATCH v1 1/3] Make type Datum be 8 bytes wide everywhere.
This still-WIP patch aims to make sizeof(Datum) be 8 on all
platforms including 32-bit ones. The objective is to allow
USE_FLOAT8_BYVAL to be true everywhere, and in consequence to
remove a lot of code that is specific to pass-by-reference
handling of float8, int8, etc. In this way we can reduce the
maintenance effort involved in supporting 32-bit platforms,
without going so far as to actually desupport them. Since Datum
is strictly an in-memory concept, this has no impact on on-disk
storage, though an initdb or pg_upgrade will be needed to fix
affected pg_type.typbyval entries.
We can expect that this change will make 32-bit builds a bit slower
and more memory-hungry, although being able to use pass-by-value
handling of 8-byte types may buy back some of that. But we stopped
optimizing for 32-bit cases a long time ago, and this seems like
just another step on that path.
This initial patch simply forces the correct type definition and
USE_FLOAT8_BYVAL setting, and cleans up a couple of minor compiler
complaints that ensued. This is sufficient for testing purposes.
In the wake of a bunch of Datum-conversion cleanups by Peter
Eisentraut, this now compiles cleanly with gcc on a 32-bit platform.
(I'd only tested the previous version with clang, which it turns out
is less picky than gcc about width-changing coercions.)
If we pursue this path, we would of course want to remove all
the now-dead code (search for mentions of SIZEOF_DATUM and
USE_FLOAT8_BYVAL to find likely places to clean up). I have
not bothered with that yet. A more interesting question is
whether there are comments or calculations that need adjustment.
There is probably an effect on the largest array that we can
support in 32-bit mode, for example.
Discussion: https://postgr.es/m/1749799.1752797397@sss.pgh.pa.us
---
src/backend/storage/ipc/ipc.c | 2 +-
src/backend/utils/resowner/resowner.c | 7 ++-----
src/include/nodes/nodes.h | 8 ++++++--
src/include/pg_config_manual.h | 13 ++++---------
src/include/postgres.h | 17 +++++++++--------
5 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index 567739b5be9..2704e80b3a7 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -399,7 +399,7 @@ cancel_before_shmem_exit(pg_on_exit_callback function, Datum arg)
before_shmem_exit_list[before_shmem_exit_index - 1].arg == arg)
--before_shmem_exit_index;
else
- elog(ERROR, "before_shmem_exit callback (%p,0x%" PRIxPTR ") is not the latest entry",
+ elog(ERROR, "before_shmem_exit callback (%p,0x%" PRIx64 ") is not the latest entry",
function, arg);
}
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index d39f3e1b655..fca84ded6dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -231,11 +231,8 @@ hash_resource_elem(Datum value, const ResourceOwnerDesc *kind)
* 'kind' into the hash. Just add it with hash_combine(), it perturbs the
* result enough for our purposes.
*/
-#if SIZEOF_DATUM == 8
- return hash_combine64(murmurhash64((uint64) value), (uint64) kind);
-#else
- return hash_combine(murmurhash32((uint32) value), (uint32) kind);
-#endif
+ return hash_combine64(murmurhash64((uint64) value),
+ (uint64) (uintptr_t) kind);
}
/*
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index fbe333d88fa..b2dc380b57b 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -188,6 +188,8 @@ castNodeImpl(NodeTag type, void *ptr)
* ----------------------------------------------------------------
*/
+#ifndef FRONTEND
+
/*
* nodes/{outfuncs.c,print.c}
*/
@@ -198,7 +200,7 @@ extern void outNode(struct StringInfoData *str, const void *obj);
extern void outToken(struct StringInfoData *str, const char *s);
extern void outBitmapset(struct StringInfoData *str,
const struct Bitmapset *bms);
-extern void outDatum(struct StringInfoData *str, uintptr_t value,
+extern void outDatum(struct StringInfoData *str, Datum value,
int typlen, bool typbyval);
extern char *nodeToString(const void *obj);
extern char *nodeToStringWithLocations(const void *obj);
@@ -212,7 +214,7 @@ extern void *stringToNode(const char *str);
extern void *stringToNodeWithLocations(const char *str);
#endif
extern struct Bitmapset *readBitmapset(void);
-extern uintptr_t readDatum(bool typbyval);
+extern Datum readDatum(bool typbyval);
extern bool *readBoolCols(int numCols);
extern int *readIntCols(int numCols);
extern Oid *readOidCols(int numCols);
@@ -235,6 +237,8 @@ extern void *copyObjectImpl(const void *from);
*/
extern bool equal(const void *a, const void *b);
+#endif /* !FRONTEND */
+
/*
* Typedef for parse location. This is just an int, but this way
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 125d3eb5fff..7e1aa422332 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -74,17 +74,12 @@
#define PARTITION_MAX_KEYS 32
/*
- * Decide whether built-in 8-byte types, including float8, int8, and
- * timestamp, are passed by value. This is on by default if sizeof(Datum) >=
- * 8 (that is, on 64-bit platforms). If sizeof(Datum) < 8 (32-bit platforms),
- * this must be off. We keep this here as an option so that it is easy to
- * test the pass-by-reference code paths on 64-bit platforms.
- *
- * Changing this requires an initdb.
+ * This symbol is now vestigial: built-in 8-byte types, including float8,
+ * int8, and timestamp, are always passed by value since we require Datum
+ * to be wide enough to permit that. We continue to define the symbol here
+ * so as not to unnecessarily break extension code.
*/
-#if SIZEOF_VOID_P >= 8
#define USE_FLOAT8_BYVAL 1
-#endif
/*
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 8a41a668687..79cf6de647d 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -58,15 +58,18 @@
/*
* A Datum contains either a value of a pass-by-value type or a pointer to a
- * value of a pass-by-reference type. Therefore, we require:
- *
- * sizeof(Datum) == sizeof(void *) == 4 or 8
+ * value of a pass-by-reference type. Therefore, we must have
+ * sizeof(Datum) >= sizeof(void *). No current or foreseeable Postgres
+ * platform has pointers wider than 8 bytes, and standardizing on Datum being
+ * exactly 8 bytes has advantages in reducing cross-platform differences.
*
* The functions below and the analogous functions for other types should be used to
* convert between a Datum and the appropriate C type.
*/
-typedef uintptr_t Datum;
+typedef uint64_t Datum;
+
+#define SIZEOF_DATUM 8
/*
* A NullableDatum is used in places where both a Datum and its nullness needs
@@ -83,8 +86,6 @@ typedef struct NullableDatum
/* due to alignment padding this could be used for flags for free */
} NullableDatum;
-#define SIZEOF_DATUM SIZEOF_VOID_P
-
/*
* DatumGetBool
* Returns boolean value of a datum.
@@ -316,7 +317,7 @@ CommandIdGetDatum(CommandId X)
static inline Pointer
DatumGetPointer(Datum X)
{
- return (Pointer) X;
+ return (Pointer) (uintptr_t) X;
}
/*
@@ -326,7 +327,7 @@ DatumGetPointer(Datum X)
static inline Datum
PointerGetDatum(const void *X)
{
- return (Datum) X;
+ return (Datum) (uintptr_t) X;
}
/*
--
2.43.7
From a7a7e9d7106d87fd416113d84ebc1e4172f2468f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 9 Aug 2025 12:04:44 -0400
Subject: [PATCH v1 2/3] Grab the low-hanging fruit from forcing sizeof(Datum)
to 8.
Remove conditionally-compiled code for smaller Datum widths.
I also fixed up a few more places that were not using
DatumGetIntXX where they should.
One thing I remembered while preparing this part is that SP-GiST
stores pass-by-value prefix keys as Datums, so that the on-disk
representation depends on sizeof(Datum). That's even more
unfortunate than the existing commentary makes it out to be,
because now there is a hazard that the change of sizeof(Datum)
will break SP-GiST indexes on 32-bit machines. It appears that
there are no existing SP-GiST opclasses that are actually
affected; and if there are some that I didn't find, the number
of installations that are using them on 32-bit machines is
doubtless tiny. So I'm proceeding on the assumption that we
can get away with this, but it's something to worry about.
(gininsert.c looks like it has a similar problem, but it's okay
because the "tuples" it's constructing are just transient data
within the tuplesort step. That's pretty poorly documented
though, so I added some comments.)
Discussion: https://postgr.es/m/1749799.1752797397@sss.pgh.pa.us
---
doc/src/sgml/xfunc.sgml | 3 +-
src/backend/access/gin/gininsert.c | 5 +-
src/backend/access/gist/gistproc.c | 10 +--
src/backend/access/nbtree/nbtcompare.c | 20 -----
src/backend/catalog/pg_type.c | 2 -
src/backend/utils/adt/mac.c | 27 +++----
src/backend/utils/adt/network.c | 8 +-
src/backend/utils/adt/numeric.c | 106 ++-----------------------
src/backend/utils/adt/timestamp.c | 21 -----
src/backend/utils/adt/uuid.c | 6 +-
src/backend/utils/adt/varlena.c | 12 +--
src/backend/utils/sort/tuplesort.c | 8 --
src/include/access/gin_tuple.h | 4 +-
src/include/access/spgist_private.h | 10 ++-
src/include/access/tupmacs.h | 4 -
src/include/port/pg_bswap.h | 15 +---
src/include/utils/sortsupport.h | 4 -
17 files changed, 44 insertions(+), 221 deletions(-)
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 30219f432d9..f116d0648e5 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -2051,8 +2051,7 @@ PG_MODULE_MAGIC_EXT(
</para>
<para>
- By-value types can only be 1, 2, or 4 bytes in length
- (also 8 bytes, if <literal>sizeof(Datum)</literal> is 8 on your machine).
+ By-value types can only be 1, 2, 4, or 8 bytes in length.
You should be careful to define your types such that they will be the
same size (in bytes) on all architectures. For example, the
<literal>long</literal> type is dangerous because it is 4 bytes on some
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 47b1898a064..e9d4b27427e 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -2189,7 +2189,10 @@ typedef struct
* we simply copy the whole Datum, so that we don't have to care about stuff
* like endianess etc. We could make it a little bit smaller, but it's not
* worth it - it's a tiny fraction of the data, and we need to MAXALIGN the
- * start of the TID list anyway. So we wouldn't save anything.
+ * start of the TID list anyway. So we wouldn't save anything. (This would
+ * not be a good idea for the permanent in-index data, since we'd prefer
+ * that that not depend on sizeof(Datum). But this is just a transient
+ * representation to use while sorting the data.)
*
* The TID list is serialized as compressed - it's highly compressible, and
* we already have ginCompressPostingList for this purpose. The list may be
diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c
index 392163cb229..a6ab7db5465 100644
--- a/src/backend/access/gist/gistproc.c
+++ b/src/backend/access/gist/gistproc.c
@@ -1707,8 +1707,8 @@ gist_bbox_zorder_cmp(Datum a, Datum b, SortSupport ssup)
* Abbreviated version of Z-order comparison
*
* The abbreviated format is a Z-order value computed from the two 32-bit
- * floats. If SIZEOF_DATUM == 8, the 64-bit Z-order value fits fully in the
- * abbreviated Datum, otherwise use its most significant bits.
+ * floats. Now that SIZEOF_DATUM is always 8, the 64-bit Z-order value
+ * always fits fully in the abbreviated Datum.
*/
static Datum
gist_bbox_zorder_abbrev_convert(Datum original, SortSupport ssup)
@@ -1718,11 +1718,7 @@ gist_bbox_zorder_abbrev_convert(Datum original, SortSupport ssup)
z = point_zorder_internal(p->x, p->y);
-#if SIZEOF_DATUM == 8
- return (Datum) z;
-#else
- return (Datum) (z >> 32);
-#endif
+ return UInt64GetDatum(z);
}
/*
diff --git a/src/backend/access/nbtree/nbtcompare.c b/src/backend/access/nbtree/nbtcompare.c
index e1b52acd20d..188c27b4925 100644
--- a/src/backend/access/nbtree/nbtcompare.c
+++ b/src/backend/access/nbtree/nbtcompare.c
@@ -278,32 +278,12 @@ btint8cmp(PG_FUNCTION_ARGS)
PG_RETURN_INT32(A_LESS_THAN_B);
}
-#if SIZEOF_DATUM < 8
-static int
-btint8fastcmp(Datum x, Datum y, SortSupport ssup)
-{
- int64 a = DatumGetInt64(x);
- int64 b = DatumGetInt64(y);
-
- if (a > b)
- return A_GREATER_THAN_B;
- else if (a == b)
- return 0;
- else
- return A_LESS_THAN_B;
-}
-#endif
-
Datum
btint8sortsupport(PG_FUNCTION_ARGS)
{
SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
-#if SIZEOF_DATUM >= 8
ssup->comparator = ssup_datum_signed_cmp;
-#else
- ssup->comparator = btint8fastcmp;
-#endif
PG_RETURN_VOID();
}
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 1ec523ee3e5..8b5c427c097 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -285,7 +285,6 @@ TypeCreate(Oid newTypeOid,
errmsg("alignment \"%c\" is invalid for passed-by-value type of size %d",
alignment, internalSize)));
}
-#if SIZEOF_DATUM == 8
else if (internalSize == (int16) sizeof(Datum))
{
if (alignment != TYPALIGN_DOUBLE)
@@ -294,7 +293,6 @@ TypeCreate(Oid newTypeOid,
errmsg("alignment \"%c\" is invalid for passed-by-value type of size %d",
alignment, internalSize)));
}
-#endif
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
diff --git a/src/backend/utils/adt/mac.c b/src/backend/utils/adt/mac.c
index 3644e9735f5..bb38ef2f5e4 100644
--- a/src/backend/utils/adt/mac.c
+++ b/src/backend/utils/adt/mac.c
@@ -481,33 +481,26 @@ macaddr_abbrev_convert(Datum original, SortSupport ssup)
Datum res;
/*
- * On a 64-bit machine, zero out the 8-byte datum and copy the 6 bytes of
- * the MAC address in. There will be two bytes of zero padding on the end
- * of the least significant bits.
+ * Zero out the 8-byte Datum and copy in the 6 bytes of the MAC address.
+ * There will be two bytes of zero padding on the end of the least
+ * significant bits.
*/
-#if SIZEOF_DATUM == 8
- memset(&res, 0, SIZEOF_DATUM);
+ StaticAssertStmt(sizeof(res) >= sizeof(macaddr),
+ "Datum is too small for macaddr");
+ memset(&res, 0, sizeof(res));
memcpy(&res, authoritative, sizeof(macaddr));
-#else /* SIZEOF_DATUM != 8 */
- memcpy(&res, authoritative, SIZEOF_DATUM);
-#endif
uss->input_count += 1;
/*
- * Cardinality estimation. The estimate uses uint32, so on a 64-bit
- * architecture, XOR the two 32-bit halves together to produce slightly
- * more entropy. The two zeroed bytes won't have any practical impact on
- * this operation.
+ * Cardinality estimation. The estimate uses uint32, so XOR the two 32-bit
+ * halves together to produce slightly more entropy. The two zeroed bytes
+ * won't have any practical impact on this operation.
*/
if (uss->estimating)
{
uint32 tmp;
-#if SIZEOF_DATUM == 8
- tmp = (uint32) res ^ (uint32) ((uint64) res >> 32);
-#else /* SIZEOF_DATUM != 8 */
- tmp = (uint32) res;
-#endif
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
}
diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c
index 9fd211b2d45..a269b36abe8 100644
--- a/src/backend/utils/adt/network.c
+++ b/src/backend/utils/adt/network.c
@@ -701,7 +701,6 @@ network_abbrev_convert(Datum original, SortSupport ssup)
network = ipaddr_datum;
}
-#if SIZEOF_DATUM == 8
if (ip_family(authoritative) == PGSQL_AF_INET)
{
/*
@@ -750,7 +749,6 @@ network_abbrev_convert(Datum original, SortSupport ssup)
res |= network | netmask_size | subnet;
}
else
-#endif
{
/*
* 4 byte datums, or IPv6 with 8 byte datums: Use as many of the
@@ -767,11 +765,7 @@ network_abbrev_convert(Datum original, SortSupport ssup)
{
uint32 tmp;
-#if SIZEOF_DATUM == 8
- tmp = (uint32) res ^ (uint32) ((uint64) res >> 32);
-#else /* SIZEOF_DATUM != 8 */
- tmp = (uint32) res;
-#endif
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 122f2efab8b..256a1a49419 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -392,30 +392,21 @@ typedef struct NumericSumAccum
/*
* We define our own macros for packing and unpacking abbreviated-key
- * representations for numeric values in order to avoid depending on
- * USE_FLOAT8_BYVAL. The type of abbreviation we use is based only on
- * the size of a datum, not the argument-passing convention for float8.
+ * representations, just to have a notational indication that that's
+ * what we're doing. Now that SIZEOF_DATUM is always 8, we can rely
+ * on fitting an int64 into Datum.
*
- * The range of abbreviations for finite values is from +PG_INT64/32_MAX
- * to -PG_INT64/32_MAX. NaN has the abbreviation PG_INT64/32_MIN, and we
+ * The range of abbreviations for finite values is from +PG_INT64_MAX
+ * to -PG_INT64_MAX. NaN has the abbreviation PG_INT64_MIN, and we
* define the sort ordering to make that work out properly (see further
* comments below). PINF and NINF share the abbreviations of the largest
* and smallest finite abbreviation classes.
*/
-#define NUMERIC_ABBREV_BITS (SIZEOF_DATUM * BITS_PER_BYTE)
-#if SIZEOF_DATUM == 8
-#define NumericAbbrevGetDatum(X) ((Datum) (X))
-#define DatumGetNumericAbbrev(X) ((int64) (X))
+#define NumericAbbrevGetDatum(X) Int64GetDatum(X)
+#define DatumGetNumericAbbrev(X) DatumGetInt64(X)
#define NUMERIC_ABBREV_NAN NumericAbbrevGetDatum(PG_INT64_MIN)
#define NUMERIC_ABBREV_PINF NumericAbbrevGetDatum(-PG_INT64_MAX)
#define NUMERIC_ABBREV_NINF NumericAbbrevGetDatum(PG_INT64_MAX)
-#else
-#define NumericAbbrevGetDatum(X) ((Datum) (X))
-#define DatumGetNumericAbbrev(X) ((int32) (X))
-#define NUMERIC_ABBREV_NAN NumericAbbrevGetDatum(PG_INT32_MIN)
-#define NUMERIC_ABBREV_PINF NumericAbbrevGetDatum(-PG_INT32_MAX)
-#define NUMERIC_ABBREV_NINF NumericAbbrevGetDatum(PG_INT32_MAX)
-#endif
/* ----------
@@ -2328,7 +2319,7 @@ numeric_cmp_abbrev(Datum x, Datum y, SortSupport ssup)
}
/*
- * Abbreviate a NumericVar according to the available bit size.
+ * Abbreviate a NumericVar into the 64-bit sortsupport size.
*
* The 31-bit value is constructed as:
*
@@ -2372,9 +2363,6 @@ numeric_cmp_abbrev(Datum x, Datum y, SortSupport ssup)
* with all bits zero. This allows simple comparisons to work on the composite
* value.
*/
-
-#if NUMERIC_ABBREV_BITS == 64
-
static Datum
numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss)
{
@@ -2426,84 +2414,6 @@ numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss)
return NumericAbbrevGetDatum(result);
}
-#endif /* NUMERIC_ABBREV_BITS == 64 */
-
-#if NUMERIC_ABBREV_BITS == 32
-
-static Datum
-numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss)
-{
- int ndigits = var->ndigits;
- int weight = var->weight;
- int32 result;
-
- if (ndigits == 0 || weight < -11)
- {
- result = 0;
- }
- else if (weight > 20)
- {
- result = PG_INT32_MAX;
- }
- else
- {
- NumericDigit nxt1 = (ndigits > 1) ? var->digits[1] : 0;
-
- weight = (weight + 11) * 4;
-
- result = var->digits[0];
-
- /*
- * "result" now has 1 to 4 nonzero decimal digits. We pack in more
- * digits to make 7 in total (largest we can fit in 24 bits)
- */
-
- if (result > 999)
- {
- /* already have 4 digits, add 3 more */
- result = (result * 1000) + (nxt1 / 10);
- weight += 3;
- }
- else if (result > 99)
- {
- /* already have 3 digits, add 4 more */
- result = (result * 10000) + nxt1;
- weight += 2;
- }
- else if (result > 9)
- {
- NumericDigit nxt2 = (ndigits > 2) ? var->digits[2] : 0;
-
- /* already have 2 digits, add 5 more */
- result = (result * 100000) + (nxt1 * 10) + (nxt2 / 1000);
- weight += 1;
- }
- else
- {
- NumericDigit nxt2 = (ndigits > 2) ? var->digits[2] : 0;
-
- /* already have 1 digit, add 6 more */
- result = (result * 1000000) + (nxt1 * 100) + (nxt2 / 100);
- }
-
- result = result | (weight << 24);
- }
-
- /* the abbrev is negated relative to the original */
- if (var->sign == NUMERIC_POS)
- result = -result;
-
- if (nss->estimating)
- {
- uint32 tmp = (uint32) result;
-
- addHyperLogLog(&nss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
- }
-
- return NumericAbbrevGetDatum(result);
-}
-
-#endif /* NUMERIC_ABBREV_BITS == 32 */
/*
* Ordinary (non-sortsupport) comparisons follow.
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index e640b48205b..3e5f9dc1458 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -2275,33 +2275,12 @@ timestamp_cmp(PG_FUNCTION_ARGS)
PG_RETURN_INT32(timestamp_cmp_internal(dt1, dt2));
}
-#if SIZEOF_DATUM < 8
-/* note: this is used for timestamptz also */
-static int
-timestamp_fastcmp(Datum x, Datum y, SortSupport ssup)
-{
- Timestamp a = DatumGetTimestamp(x);
- Timestamp b = DatumGetTimestamp(y);
-
- return timestamp_cmp_internal(a, b);
-}
-#endif
-
Datum
timestamp_sortsupport(PG_FUNCTION_ARGS)
{
SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
-#if SIZEOF_DATUM >= 8
-
- /*
- * If this build has pass-by-value timestamps, then we can use a standard
- * comparator function.
- */
ssup->comparator = ssup_datum_signed_cmp;
-#else
- ssup->comparator = timestamp_fastcmp;
-#endif
PG_RETURN_VOID();
}
diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index bce7309c183..7413239f7af 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -398,11 +398,7 @@ uuid_abbrev_convert(Datum original, SortSupport ssup)
{
uint32 tmp;
-#if SIZEOF_DATUM == 8
- tmp = (uint32) res ^ (uint32) ((uint64) res >> 32);
-#else /* SIZEOF_DATUM != 8 */
- tmp = (uint32) res;
-#endif
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
}
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 11b442a5941..4fc3f0886a4 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2132,18 +2132,12 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
addHyperLogLog(&sss->full_card, hash);
/* Hash abbreviated key */
-#if SIZEOF_DATUM == 8
{
- uint32 lohalf,
- hihalf;
+ uint32 tmp;
- lohalf = (uint32) res;
- hihalf = (uint32) (res >> 32);
- hash = DatumGetUInt32(hash_uint32(lohalf ^ hihalf));
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
+ hash = DatumGetUInt32(hash_uint32(tmp));
}
-#else /* SIZEOF_DATUM != 8 */
- hash = DatumGetUInt32(hash_uint32((uint32) res));
-#endif
addHyperLogLog(&sss->abbr_card, hash);
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 65ab83fff8b..5d4411dc33f 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -512,7 +512,6 @@ qsort_tuple_unsigned_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
return state->base.comparetup_tiebreak(a, b, state);
}
-#if SIZEOF_DATUM >= 8
/* Used if first key's comparator is ssup_datum_signed_cmp */
static pg_attribute_always_inline int
qsort_tuple_signed_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
@@ -535,7 +534,6 @@ qsort_tuple_signed_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
return state->base.comparetup_tiebreak(a, b, state);
}
-#endif
/* Used if first key's comparator is ssup_datum_int32_cmp */
static pg_attribute_always_inline int
@@ -578,7 +576,6 @@ qsort_tuple_int32_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
#define ST_DEFINE
#include "lib/sort_template.h"
-#if SIZEOF_DATUM >= 8
#define ST_SORT qsort_tuple_signed
#define ST_ELEMENT_TYPE SortTuple
#define ST_COMPARE(a, b, state) qsort_tuple_signed_compare(a, b, state)
@@ -587,7 +584,6 @@ qsort_tuple_int32_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
#define ST_SCOPE static
#define ST_DEFINE
#include "lib/sort_template.h"
-#endif
#define ST_SORT qsort_tuple_int32
#define ST_ELEMENT_TYPE SortTuple
@@ -2692,7 +2688,6 @@ tuplesort_sort_memtuples(Tuplesortstate *state)
state);
return;
}
-#if SIZEOF_DATUM >= 8
else if (state->base.sortKeys[0].comparator == ssup_datum_signed_cmp)
{
qsort_tuple_signed(state->memtuples,
@@ -2700,7 +2695,6 @@ tuplesort_sort_memtuples(Tuplesortstate *state)
state);
return;
}
-#endif
else if (state->base.sortKeys[0].comparator == ssup_datum_int32_cmp)
{
qsort_tuple_int32(state->memtuples,
@@ -3146,7 +3140,6 @@ ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup)
return 0;
}
-#if SIZEOF_DATUM >= 8
int
ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup)
{
@@ -3160,7 +3153,6 @@ ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup)
else
return 0;
}
-#endif
int
ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup)
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 702f7d12889..b4f103dec9a 100644
--- a/src/include/access/gin_tuple.h
+++ b/src/include/access/gin_tuple.h
@@ -15,7 +15,9 @@
#include "utils/sortsupport.h"
/*
- * Data for one key in a GIN index.
+ * Data for one key in a GIN index. (This is not the permanent in-index
+ * representation, but just a convenient format to use during the tuplesort
+ * stage of building a new GIN index.)
*/
typedef struct GinTuple
{
diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h
index cb43a278f46..fdda7f4c639 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -285,10 +285,12 @@ typedef struct SpGistCache
* If the prefix datum is of a pass-by-value type, it is stored in its
* Datum representation, that is its on-disk representation is of length
* sizeof(Datum). This is a fairly unfortunate choice, because in no other
- * place does Postgres use Datum as an on-disk representation; it creates
- * an unnecessary incompatibility between 32-bit and 64-bit builds. But the
- * compatibility loss is mostly theoretical since MAXIMUM_ALIGNOF typically
- * differs between such builds, too. Anyway we're stuck with it now.
+ * place does Postgres use Datum as an on-disk representation. Formerly it
+ * meant an unnecessary incompatibility between 32-bit and 64-bit builds, and
+ * as of v19 it instead creates a hazard for binary upgrades on 32-bit builds.
+ * Fortunately, that hazard seems mostly theoretical for lack of affected
+ * opclasses. Going forward, we will be using a fixed size of Datum so that
+ * there's no longer any pressing reason to change this.
*/
typedef struct SpGistInnerTupleData
{
diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h
index 6240ec930e7..7ef006fa7eb 100644
--- a/src/include/access/tupmacs.h
+++ b/src/include/access/tupmacs.h
@@ -62,10 +62,8 @@ fetch_att(const void *T, bool attbyval, int attlen)
return Int16GetDatum(*((const int16 *) T));
case sizeof(int32):
return Int32GetDatum(*((const int32 *) T));
-#if SIZEOF_DATUM == 8
case sizeof(Datum):
return *((const Datum *) T);
-#endif
default:
elog(ERROR, "unsupported byval length: %d", attlen);
return 0;
@@ -221,11 +219,9 @@ store_att_byval(void *T, Datum newdatum, int attlen)
case sizeof(int32):
*(int32 *) T = DatumGetInt32(newdatum);
break;
-#if SIZEOF_DATUM == 8
case sizeof(Datum):
*(Datum *) T = newdatum;
break;
-#endif
default:
elog(ERROR, "unsupported byval length: %d", attlen);
}
diff --git a/src/include/port/pg_bswap.h b/src/include/port/pg_bswap.h
index 33648433c63..f823588f58f 100644
--- a/src/include/port/pg_bswap.h
+++ b/src/include/port/pg_bswap.h
@@ -130,8 +130,7 @@ pg_bswap64(uint64 x)
/*
* Rearrange the bytes of a Datum from big-endian order into the native byte
- * order. On big-endian machines, this does nothing at all. Note that the C
- * type Datum is an unsigned integer type on all platforms.
+ * order. On big-endian machines, this does nothing at all.
*
* One possible application of the DatumBigEndianToNative() macro is to make
* bitwise comparisons cheaper. A simple 3-way comparison of Datums
@@ -141,20 +140,14 @@ pg_bswap64(uint64 x)
* without any special transformation occurring first.
*
* If SIZEOF_DATUM is not defined, then postgres.h wasn't included and these
- * macros probably shouldn't be used, so we define nothing. Note that
- * SIZEOF_DATUM == 8 would evaluate as 0 == 8 in that case, potentially
- * leading to the wrong implementation being selected and confusing errors, so
- * defining nothing is safest.
+ * macros probably shouldn't be used, so we define nothing. (DatumGetUInt64
+ * and UInt64GetDatum won't be available either.)
*/
#ifdef SIZEOF_DATUM
#ifdef WORDS_BIGENDIAN
#define DatumBigEndianToNative(x) (x)
#else /* !WORDS_BIGENDIAN */
-#if SIZEOF_DATUM == 8
-#define DatumBigEndianToNative(x) pg_bswap64(x)
-#else /* SIZEOF_DATUM != 8 */
-#define DatumBigEndianToNative(x) pg_bswap32(x)
-#endif /* SIZEOF_DATUM == 8 */
+#define DatumBigEndianToNative(x) UInt64GetDatum(pg_bswap64(DatumGetUInt64(x)))
#endif /* WORDS_BIGENDIAN */
#endif /* SIZEOF_DATUM */
diff --git a/src/include/utils/sortsupport.h b/src/include/utils/sortsupport.h
index b7abaf7802d..c64527e2ee9 100644
--- a/src/include/utils/sortsupport.h
+++ b/src/include/utils/sortsupport.h
@@ -262,7 +262,6 @@ ApplyUnsignedSortComparator(Datum datum1, bool isNull1,
return compare;
}
-#if SIZEOF_DATUM >= 8
static inline int
ApplySignedSortComparator(Datum datum1, bool isNull1,
Datum datum2, bool isNull2,
@@ -296,7 +295,6 @@ ApplySignedSortComparator(Datum datum1, bool isNull1,
return compare;
}
-#endif
static inline int
ApplyInt32SortComparator(Datum datum1, bool isNull1,
@@ -376,9 +374,7 @@ ApplySortAbbrevFullComparator(Datum datum1, bool isNull1,
* are eligible for faster sorting.
*/
extern int ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup);
-#if SIZEOF_DATUM >= 8
extern int ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup);
-#endif
extern int ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup);
/* Other functions in utils/sort/sortsupport.c */
--
2.43.7
From 4a1999f8758b01582d7695011880183f6fb75304 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 9 Aug 2025 12:35:48 -0400
Subject: [PATCH v1 3/3] Grab the low-hanging fruit from forcing
USE_FLOAT8_BYVAL to true.
Remove conditionally-compiled code for the other case.
I did not bother removing uses of the FLOAT8PASSBYVAL macro,
except for getting rid of it in pg_type.dat so that initdb can
skip the step of replacing it. Other references could be
replaced with "true" but it would save nothing.
I also left the associated pg_control and Pg_magic_struct
fields in place. Perhaps we should get rid of them, but
it would save little.
Discussion: https://postgr.es/m/1749799.1752797397@sss.pgh.pa.us
---
contrib/btree_gist/btree_time.c | 51 +++++++++-----------
contrib/btree_gist/btree_ts.c | 39 +++++++---------
src/backend/access/index/indexam.c | 5 --
src/backend/access/transam/xlog.c | 10 ----
src/backend/utils/adt/int8.c | 75 ++++++------------------------
src/backend/utils/adt/numeric.c | 74 +++++++----------------------
src/backend/utils/fmgr/fmgr.c | 35 --------------
src/bin/initdb/initdb.c | 3 --
src/include/c.h | 8 ++--
src/include/catalog/pg_type.dat | 16 +++----
src/include/postgres.h | 58 ++---------------------
11 files changed, 84 insertions(+), 290 deletions(-)
diff --git a/contrib/btree_gist/btree_time.c b/contrib/btree_gist/btree_time.c
index 1dba95057ba..fd6a2e05bc7 100644
--- a/contrib/btree_gist/btree_time.c
+++ b/contrib/btree_gist/btree_time.c
@@ -31,13 +31,6 @@ PG_FUNCTION_INFO_V1(gbt_time_sortsupport);
PG_FUNCTION_INFO_V1(gbt_timetz_sortsupport);
-#ifdef USE_FLOAT8_BYVAL
-#define TimeADTGetDatumFast(X) TimeADTGetDatum(X)
-#else
-#define TimeADTGetDatumFast(X) PointerGetDatum(&(X))
-#endif
-
-
static bool
gbt_timegt(const void *a, const void *b, FmgrInfo *flinfo)
{
@@ -45,8 +38,8 @@ gbt_timegt(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_gt,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -56,8 +49,8 @@ gbt_timege(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_ge,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -67,8 +60,8 @@ gbt_timeeq(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_eq,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -78,8 +71,8 @@ gbt_timele(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_le,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -89,8 +82,8 @@ gbt_timelt(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_lt,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static int
@@ -100,9 +93,9 @@ gbt_timekey_cmp(const void *a, const void *b, FmgrInfo *flinfo)
timeKEY *ib = (timeKEY *) (((const Nsrt *) b)->t);
int res;
- res = DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatumFast(ia->lower),
TimeADTGetDatumFast(ib->lower)));
+ res = DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatum(ia->lower), TimeADTGetDatum(ib->lower)));
if (res == 0)
- return DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatumFast(ia->upper),
TimeADTGetDatumFast(ib->upper)));
+ return DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatum(ia->upper), TimeADTGetDatum(ib->upper)));
return res;
}
@@ -115,8 +108,8 @@ gbt_time_dist(const void *a, const void *b, FmgrInfo *flinfo)
Interval *i;
i = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
return fabs(INTERVAL_TO_SEC(i));
}
@@ -279,14 +272,14 @@ gbt_time_penalty(PG_FUNCTION_ARGS)
double res2;
intr = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(newentry->upper),
- TimeADTGetDatumFast(origentry->upper)));
+ TimeADTGetDatum(newentry->upper),
+ TimeADTGetDatum(origentry->upper)));
res = INTERVAL_TO_SEC(intr);
res = Max(res, 0);
intr = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(origentry->lower),
- TimeADTGetDatumFast(newentry->lower)));
+ TimeADTGetDatum(origentry->lower),
+ TimeADTGetDatum(newentry->lower)));
res2 = INTERVAL_TO_SEC(intr);
res2 = Max(res2, 0);
@@ -297,8 +290,8 @@ gbt_time_penalty(PG_FUNCTION_ARGS)
if (res > 0)
{
intr = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(origentry->upper),
- TimeADTGetDatumFast(origentry->lower)));
+ TimeADTGetDatum(origentry->upper),
+ TimeADTGetDatum(origentry->lower)));
*result += FLT_MIN;
*result += (float) (res / (res + INTERVAL_TO_SEC(intr)));
*result *= (FLT_MAX / (((GISTENTRY *) PG_GETARG_POINTER(0))->rel->rd_att->natts + 1));
@@ -334,8 +327,8 @@ gbt_timekey_ssup_cmp(Datum x, Datum y, SortSupport ssup)
/* for leaf items we expect lower == upper, so only compare lower */
return DatumGetInt32(DirectFunctionCall2(time_cmp,
- TimeADTGetDatumFast(arg1->lower),
- TimeADTGetDatumFast(arg2->lower)));
+ TimeADTGetDatum(arg1->lower),
+ TimeADTGetDatum(arg2->lower)));
}
Datum
diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index eb899c4d213..1e8f83f0f0a 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -33,13 +33,6 @@ PG_FUNCTION_INFO_V1(gbt_ts_same);
PG_FUNCTION_INFO_V1(gbt_ts_sortsupport);
-#ifdef USE_FLOAT8_BYVAL
-#define TimestampGetDatumFast(X) TimestampGetDatum(X)
-#else
-#define TimestampGetDatumFast(X) PointerGetDatum(&(X))
-#endif
-
-
/* define for comparison */
static bool
@@ -49,8 +42,8 @@ gbt_tsgt(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_gt,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -60,8 +53,8 @@ gbt_tsge(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_ge,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -71,8 +64,8 @@ gbt_tseq(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_eq,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -82,8 +75,8 @@ gbt_tsle(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_le,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -93,8 +86,8 @@ gbt_tslt(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_lt,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static int
@@ -104,9 +97,9 @@ gbt_tskey_cmp(const void *a, const void *b, FmgrInfo *flinfo)
tsKEY *ib = (tsKEY *) (((const Nsrt *) b)->t);
int res;
- res = DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatumFast(ia->lower),
TimestampGetDatumFast(ib->lower)));
+ res = DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatum(ia->lower),
TimestampGetDatum(ib->lower)));
if (res == 0)
- return DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatumFast(ia->upper),
TimestampGetDatumFast(ib->upper)));
+ return DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatum(ia->upper),
TimestampGetDatum(ib->upper)));
return res;
}
@@ -122,8 +115,8 @@ gbt_ts_dist(const void *a, const void *b, FmgrInfo *flinfo)
return get_float8_infinity();
i = DatumGetIntervalP(DirectFunctionCall2(timestamp_mi,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
return fabs(INTERVAL_TO_SEC(i));
}
@@ -404,8 +397,8 @@ gbt_ts_ssup_cmp(Datum x, Datum y, SortSupport ssup)
/* for leaf items we expect lower == upper, so only compare lower */
return DatumGetInt32(DirectFunctionCall2(timestamp_cmp,
- TimestampGetDatumFast(arg1->lower),
- TimestampGetDatumFast(arg2->lower)));
+ TimestampGetDatum(arg1->lower),
+ TimestampGetDatum(arg2->lower)));
}
Datum
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 219df1971da..1a4f36fe0a9 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -986,11 +986,6 @@ index_store_float8_orderby_distances(IndexScanDesc scan, Oid *orderByTypes,
{
if (orderByTypes[i] == FLOAT8OID)
{
-#ifndef USE_FLOAT8_BYVAL
- /* must free any old value to avoid memory leakage */
- if (!scan->xs_orderbynulls[i])
- pfree(DatumGetPointer(scan->xs_orderbyvals[i]));
-#endif
if (distances && !distances[i].isnull)
{
scan->xs_orderbyvals[i] = Float8GetDatum(distances[i].value);
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 9a4de1616bc..d4a1cfb2d03 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4651,7 +4651,6 @@ ReadControlFile(void)
"LOBLKSIZE", (int) LOBLKSIZE),
errhint("It looks like you need to recompile or initdb.")));
-#ifdef USE_FLOAT8_BYVAL
if (ControlFile->float8ByVal != true)
ereport(FATAL,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -4659,15 +4658,6 @@ ReadControlFile(void)
errdetail("The database cluster was initialized without USE_FLOAT8_BYVAL"
" but the server was compiled with USE_FLOAT8_BYVAL."),
errhint("It looks like you need to recompile or initdb.")));
-#else
- if (ControlFile->float8ByVal != false)
- ereport(FATAL,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("database files are incompatible with server"),
- errdetail("The database cluster was initialized with USE_FLOAT8_BYVAL"
- " but the server was compiled without USE_FLOAT8_BYVAL."),
- errhint("It looks like you need to recompile or initdb.")));
-#endif
wal_segment_size = ControlFile->xlog_seg_size;
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 9dd5889f34c..bdea490202a 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -718,76 +718,29 @@ int8lcm(PG_FUNCTION_ARGS)
Datum
int8inc(PG_FUNCTION_ARGS)
{
- /*
- * When int8 is pass-by-reference, we provide this special case to avoid
- * palloc overhead for COUNT(): when called as an aggregate, we know that
- * the argument is modifiable local storage, so just update it in-place.
- * (If int8 is pass-by-value, then of course this is useless as well as
- * incorrect, so just ifdef it out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *arg = (int64 *) PG_GETARG_POINTER(0);
-
- if (unlikely(pg_add_s64_overflow(*arg, 1, arg)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
-
- PG_RETURN_POINTER(arg);
- }
- else
-#endif
- {
- /* Not called as an aggregate, so just do it the dumb way */
- int64 arg = PG_GETARG_INT64(0);
- int64 result;
+ int64 arg = PG_GETARG_INT64(0);
+ int64 result;
- if (unlikely(pg_add_s64_overflow(arg, 1, &result)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
+ if (unlikely(pg_add_s64_overflow(arg, 1, &result)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
- PG_RETURN_INT64(result);
- }
+ PG_RETURN_INT64(result);
}
Datum
int8dec(PG_FUNCTION_ARGS)
{
- /*
- * When int8 is pass-by-reference, we provide this special case to avoid
- * palloc overhead for COUNT(): when called as an aggregate, we know that
- * the argument is modifiable local storage, so just update it in-place.
- * (If int8 is pass-by-value, then of course this is useless as well as
- * incorrect, so just ifdef it out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *arg = (int64 *) PG_GETARG_POINTER(0);
-
- if (unlikely(pg_sub_s64_overflow(*arg, 1, arg)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
- PG_RETURN_POINTER(arg);
- }
- else
-#endif
- {
- /* Not called as an aggregate, so just do it the dumb way */
- int64 arg = PG_GETARG_INT64(0);
- int64 result;
+ int64 arg = PG_GETARG_INT64(0);
+ int64 result;
- if (unlikely(pg_sub_s64_overflow(arg, 1, &result)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
+ if (unlikely(pg_sub_s64_overflow(arg, 1, &result)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
- PG_RETURN_INT64(result);
- }
+ PG_RETURN_INT64(result);
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 256a1a49419..9b6a5db54f2 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6363,6 +6363,7 @@ numeric_poly_stddev_pop(PG_FUNCTION_ARGS)
Datum
int2_sum(PG_FUNCTION_ARGS)
{
+ int64 oldsum;
int64 newval;
if (PG_ARGISNULL(0))
@@ -6375,43 +6376,22 @@ int2_sum(PG_FUNCTION_ARGS)
PG_RETURN_INT64(newval);
}
- /*
- * If we're invoked as an aggregate, we can cheat and modify our first
- * parameter in-place to avoid palloc overhead. If not, we need to return
- * the new value of the transition variable. (If int8 is pass-by-value,
- * then of course this is useless as well as incorrect, so just ifdef it
- * out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *oldsum = (int64 *) PG_GETARG_POINTER(0);
+ oldsum = PG_GETARG_INT64(0);
- /* Leave the running sum unchanged in the new input is null */
- if (!PG_ARGISNULL(1))
- *oldsum = *oldsum + (int64) PG_GETARG_INT16(1);
-
- PG_RETURN_POINTER(oldsum);
- }
- else
-#endif
- {
- int64 oldsum = PG_GETARG_INT64(0);
-
- /* Leave sum unchanged if new input is null. */
- if (PG_ARGISNULL(1))
- PG_RETURN_INT64(oldsum);
+ /* Leave sum unchanged if new input is null. */
+ if (PG_ARGISNULL(1))
+ PG_RETURN_INT64(oldsum);
- /* OK to do the addition. */
- newval = oldsum + (int64) PG_GETARG_INT16(1);
+ /* OK to do the addition. */
+ newval = oldsum + (int64) PG_GETARG_INT16(1);
- PG_RETURN_INT64(newval);
- }
+ PG_RETURN_INT64(newval);
}
Datum
int4_sum(PG_FUNCTION_ARGS)
{
+ int64 oldsum;
int64 newval;
if (PG_ARGISNULL(0))
@@ -6424,38 +6404,16 @@ int4_sum(PG_FUNCTION_ARGS)
PG_RETURN_INT64(newval);
}
- /*
- * If we're invoked as an aggregate, we can cheat and modify our first
- * parameter in-place to avoid palloc overhead. If not, we need to return
- * the new value of the transition variable. (If int8 is pass-by-value,
- * then of course this is useless as well as incorrect, so just ifdef it
- * out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *oldsum = (int64 *) PG_GETARG_POINTER(0);
+ oldsum = PG_GETARG_INT64(0);
- /* Leave the running sum unchanged in the new input is null */
- if (!PG_ARGISNULL(1))
- *oldsum = *oldsum + (int64) PG_GETARG_INT32(1);
-
- PG_RETURN_POINTER(oldsum);
- }
- else
-#endif
- {
- int64 oldsum = PG_GETARG_INT64(0);
-
- /* Leave sum unchanged if new input is null. */
- if (PG_ARGISNULL(1))
- PG_RETURN_INT64(oldsum);
+ /* Leave sum unchanged if new input is null. */
+ if (PG_ARGISNULL(1))
+ PG_RETURN_INT64(oldsum);
- /* OK to do the addition. */
- newval = oldsum + (int64) PG_GETARG_INT32(1);
+ /* OK to do the addition. */
+ newval = oldsum + (int64) PG_GETARG_INT32(1);
- PG_RETURN_INT64(newval);
- }
+ PG_RETURN_INT64(newval);
}
/*
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 782291d9998..5543440a33e 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -1788,41 +1788,6 @@ OidSendFunctionCall(Oid functionId, Datum val)
}
-/*-------------------------------------------------------------------------
- * Support routines for standard maybe-pass-by-reference datatypes
- *
- * int8 and float8 can be passed by value if Datum is wide enough.
- * (For backwards-compatibility reasons, we allow pass-by-ref to be chosen
- * at compile time even if pass-by-val is possible.)
- *
- * Note: there is only one switch controlling the pass-by-value option for
- * both int8 and float8; this is to avoid making things unduly complicated
- * for the timestamp types, which might have either representation.
- *-------------------------------------------------------------------------
- */
-
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
-
-Datum
-Int64GetDatum(int64 X)
-{
- int64 *retval = (int64 *) palloc(sizeof(int64));
-
- *retval = X;
- return PointerGetDatum(retval);
-}
-
-Datum
-Float8GetDatum(float8 X)
-{
- float8 *retval = (float8 *) palloc(sizeof(float8));
-
- *retval = X;
- return PointerGetDatum(retval);
-}
-#endif /* USE_FLOAT8_BYVAL */
-
-
/*-------------------------------------------------------------------------
* Support routines for toastable datatypes
*-------------------------------------------------------------------------
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 62bbd08d9f6..92fe2f531f7 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1580,9 +1580,6 @@ bootstrap_template1(void)
bki_lines = replace_token(bki_lines, "ALIGNOF_POINTER",
(sizeof(Pointer) == 4) ? "i" : "d");
- bki_lines = replace_token(bki_lines, "FLOAT8PASSBYVAL",
- FLOAT8PASSBYVAL ? "true" : "false");
-
bki_lines = replace_token(bki_lines, "POSTGRES",
escape_quotes_bki(username));
diff --git a/src/include/c.h b/src/include/c.h
index bbdaa88c63a..39022f8a9dd 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -609,11 +609,11 @@ typedef signed int Offset;
typedef float float4;
typedef double float8;
-#ifdef USE_FLOAT8_BYVAL
+/*
+ * float8, int8, and related datatypes are now always pass-by-value.
+ * We keep this symbol to avoid breaking extension code that may use it.
+ */
#define FLOAT8PASSBYVAL true
-#else
-#define FLOAT8PASSBYVAL false
-#endif
/*
* Oid, RegProcedure, TransactionId, SubTransactionId, MultiXactId,
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index 29e4ffffc98..cb730aeac86 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -54,7 +54,7 @@
typcollation => 'C' },
{ oid => '20', array_type_oid => '1016',
descr => '~18 digit integer, 8-byte storage',
- typname => 'int8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'int8', typlen => '8', typbyval => 't',
typcategory => 'N', typinput => 'int8in', typoutput => 'int8out',
typreceive => 'int8recv', typsend => 'int8send', typalign => 'd' },
{ oid => '21', array_type_oid => '1005',
@@ -172,7 +172,7 @@
typoutput => 'pg_ddl_command_out', typreceive => 'pg_ddl_command_recv',
typsend => 'pg_ddl_command_send', typalign => 'ALIGNOF_POINTER' },
{ oid => '5069', array_type_oid => '271', descr => 'full transaction id',
- typname => 'xid8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'xid8', typlen => '8', typbyval => 't',
typcategory => 'U', typinput => 'xid8in', typoutput => 'xid8out',
typreceive => 'xid8recv', typsend => 'xid8send', typalign => 'd' },
@@ -222,7 +222,7 @@
typsend => 'float4send', typalign => 'i' },
{ oid => '701', array_type_oid => '1022',
descr => 'double-precision floating point number, 8-byte storage',
- typname => 'float8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'float8', typlen => '8', typbyval => 't',
typcategory => 'N', typispreferred => 't', typinput => 'float8in',
typoutput => 'float8out', typreceive => 'float8recv', typsend => 'float8send',
typalign => 'd' },
@@ -237,7 +237,7 @@
typreceive => 'circle_recv', typsend => 'circle_send', typalign => 'd' },
{ oid => '790', array_type_oid => '791',
descr => 'monetary amounts, $d,ddd.cc',
- typname => 'money', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'money', typlen => '8', typbyval => 't',
typcategory => 'N', typinput => 'cash_in', typoutput => 'cash_out',
typreceive => 'cash_recv', typsend => 'cash_send', typalign => 'd' },
@@ -290,7 +290,7 @@
typinput => 'date_in', typoutput => 'date_out', typreceive => 'date_recv',
typsend => 'date_send', typalign => 'i' },
{ oid => '1083', array_type_oid => '1183', descr => 'time of day',
- typname => 'time', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'time', typlen => '8', typbyval => 't',
typcategory => 'D', typinput => 'time_in', typoutput => 'time_out',
typreceive => 'time_recv', typsend => 'time_send', typmodin => 'timetypmodin',
typmodout => 'timetypmodout', typalign => 'd' },
@@ -298,14 +298,14 @@
# OIDS 1100 - 1199
{ oid => '1114', array_type_oid => '1115', descr => 'date and time',
- typname => 'timestamp', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'timestamp', typlen => '8', typbyval => 't',
typcategory => 'D', typinput => 'timestamp_in', typoutput => 'timestamp_out',
typreceive => 'timestamp_recv', typsend => 'timestamp_send',
typmodin => 'timestamptypmodin', typmodout => 'timestamptypmodout',
typalign => 'd' },
{ oid => '1184', array_type_oid => '1185',
descr => 'date and time with time zone',
- typname => 'timestamptz', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'timestamptz', typlen => '8', typbyval => 't',
typcategory => 'D', typispreferred => 't', typinput => 'timestamptz_in',
typoutput => 'timestamptz_out', typreceive => 'timestamptz_recv',
typsend => 'timestamptz_send', typmodin => 'timestamptztypmodin',
@@ -413,7 +413,7 @@
# pg_lsn
{ oid => '3220', array_type_oid => '3221', descr => 'PostgreSQL LSN',
- typname => 'pg_lsn', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'pg_lsn', typlen => '8', typbyval => 't',
typcategory => 'U', typinput => 'pg_lsn_in', typoutput => 'pg_lsn_out',
typreceive => 'pg_lsn_recv', typsend => 'pg_lsn_send', typalign => 'd' },
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 79cf6de647d..00df337da90 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -384,68 +384,41 @@ NameGetDatum(const NameData *X)
/*
* DatumGetInt64
* Returns 64-bit integer value of a datum.
- *
- * Note: this function hides whether int64 is pass by value or by reference.
*/
static inline int64
DatumGetInt64(Datum X)
{
-#ifdef USE_FLOAT8_BYVAL
return (int64) X;
-#else
- return *((int64 *) DatumGetPointer(X));
-#endif
}
/*
* Int64GetDatum
* Returns datum representation for a 64-bit integer.
- *
- * Note: if int64 is pass by reference, this function returns a reference
- * to palloc'd space.
*/
-#ifdef USE_FLOAT8_BYVAL
static inline Datum
Int64GetDatum(int64 X)
{
return (Datum) X;
}
-#else
-extern Datum Int64GetDatum(int64 X);
-#endif
-
/*
* DatumGetUInt64
* Returns 64-bit unsigned integer value of a datum.
- *
- * Note: this function hides whether int64 is pass by value or by reference.
*/
static inline uint64
DatumGetUInt64(Datum X)
{
-#ifdef USE_FLOAT8_BYVAL
return (uint64) X;
-#else
- return *((uint64 *) DatumGetPointer(X));
-#endif
}
/*
* UInt64GetDatum
* Returns datum representation for a 64-bit unsigned integer.
- *
- * Note: if int64 is pass by reference, this function returns a reference
- * to palloc'd space.
*/
static inline Datum
UInt64GetDatum(uint64 X)
{
-#ifdef USE_FLOAT8_BYVAL
return (Datum) X;
-#else
- return Int64GetDatum((int64) X);
-#endif
}
/*
@@ -493,13 +466,10 @@ Float4GetDatum(float4 X)
/*
* DatumGetFloat8
* Returns 8-byte floating point value of a datum.
- *
- * Note: this function hides whether float8 is pass by value or by reference.
*/
static inline float8
DatumGetFloat8(Datum X)
{
-#ifdef USE_FLOAT8_BYVAL
union
{
int64 value;
@@ -508,19 +478,12 @@ DatumGetFloat8(Datum X)
myunion.value = DatumGetInt64(X);
return myunion.retval;
-#else
- return *((float8 *) DatumGetPointer(X));
-#endif
}
/*
* Float8GetDatum
* Returns datum representation for an 8-byte floating point number.
- *
- * Note: if float8 is pass by reference, this function returns a reference
- * to palloc'd space.
*/
-#ifdef USE_FLOAT8_BYVAL
static inline Datum
Float8GetDatum(float8 X)
{
@@ -533,35 +496,22 @@ Float8GetDatum(float8 X)
myunion.value = X;
return Int64GetDatum(myunion.retval);
}
-#else
-extern Datum Float8GetDatum(float8 X);
-#endif
-
/*
* Int64GetDatumFast
* Float8GetDatumFast
*
- * These macros are intended to allow writing code that does not depend on
+ * These macros were intended to allow writing code that does not depend on
* whether int64 and float8 are pass-by-reference types, while not
- * sacrificing performance when they are. The argument must be a variable
- * that will exist and have the same value for as long as the Datum is needed.
- * In the pass-by-ref case, the address of the variable is taken to use as
- * the Datum. In the pass-by-val case, these are the same as the non-Fast
- * functions, except for asserting that the variable is of the correct type.
+ * sacrificing performance when they are. They are no longer different
+ * from the regular functions, though we keep the assertions to protect
+ * code that might get back-patched into older branches.
*/
-#ifdef USE_FLOAT8_BYVAL
#define Int64GetDatumFast(X) \
(AssertVariableIsOfTypeMacro(X, int64), Int64GetDatum(X))
#define Float8GetDatumFast(X) \
(AssertVariableIsOfTypeMacro(X, double), Float8GetDatum(X))
-#else
-#define Int64GetDatumFast(X) \
- (AssertVariableIsOfTypeMacro(X, int64), PointerGetDatum(&(X)))
-#define Float8GetDatumFast(X) \
- (AssertVariableIsOfTypeMacro(X, double), PointerGetDatum(&(X)))
-#endif
/* ----------------------------------------------------------------
--
2.43.7
On 2025-08-09 Sa 7:45 AM, Joe Conway wrote: > On 8/8/25 21:14, Tom Lane wrote: >> I have just realized that this proposal has a rather nasty defect. >> Per the following comment in spgist_private.h: >> >> * If the prefix datum is of a pass-by-value type, it is stored in its >> * Datum representation, that is its on-disk representation is of >> length >> * sizeof(Datum). This is a fairly unfortunate choice, because in >> no other >> * place does Postgres use Datum as an on-disk representation; it >> creates >> * an unnecessary incompatibility between 32-bit and 64-bit builds. >> But the >> * compatibility loss is mostly theoretical since MAXIMUM_ALIGNOF >> typically >> * differs between such builds, too. Anyway we're stuck with it now. >> >> This means we cannot change sizeof(Datum), nor reconsider the >> pass-by-value classification of any datatype, without potentially >> breaking pg_upgrade of some SP-GiST indexes on 32-bit machines. >> >> Now, it looks like this doesn't affect any in-core SP-GiST opclasses. >> The only one using a potentially affected type is kd_point_ops which >> uses a float8 prefix. That'll have been stored in regular on-disk >> format on a 32-bit machine, but if we redefine it as being stored >> in 64-bit-Datum format, nothing actually changes. The case that >> would be problematic is a prefix type that's 4 bytes or less, and >> I don't see any. >> >> A quick search of Debian Code Search doesn't find any extensions >> that look like they are using small pass-by-value prefixes either. >> So maybe we can get away with just changing this, but it's worrisome. >> >> On the positive side, even if there are any SP-GiST opclasses that >> are at risk, the population of installations using them on 32-bit >> installs has got to be pretty tiny. > > I bet it is indistinguishable from zero... > >> And the worst-case answer is that you'd have to reindex such indexes >> after pg_upgrade. > > ...and this seems like a reasonable answer if anyone pops up. > >> BTW, I don't think we can teach pg_upgrade to check for this >> hazard, because the SP-GiST APIs are such that the data type >> used for prefixes isn't visible at the SQL level. >> >> Do we think that making this change is valuable enough to justify >> taking such a risk? > > yes +1 > > Agree to all the above cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 09.08.25 18:44, Tom Lane wrote:
> I've now fleshed out the patch series with some cleanup of code that's
> been rendered dead.
These patches look mechanically correct to me.
For patch 0002:
The various additional uses of *GetDatum and DatumGet* could be applied
as a separate patch. That would make the remaining patch clearer as
mostly just removing dead code.
In tupmacs.h, I think the two sites with
case sizeof(Datum):
should be rewritten using
case sizeof(int64):
to match the other cases. Otherwise, this code ends up looking
mysteriously asymmetric. (And the related code in pg_type.c as well.)
The remaining mentions of "SIZEOF_DATUM" in gistproc.c and network.c
could be replaced by "sizeof(Datum)". Then we could eventually remove
SIZEOF_DATUM altogether. (Maybe DatumBigEndianToNative() would better
live in a different header file at that point, not sure.)
For patch 0003:
I would also remove most of the remaining uses of FLOAT8PASSBYVAL,
especially where it is used in relation with INT8OID, which is just
endlessly confusing.
We should also get rid of these things in the control file and the ABI
magic structs, but that could be done as separate patches. It would
probably require a separate round of thinking about compatibility,
upgrading, etc.
I'm also thinking, as a follow-on project, we could get rid of typbyval
and require that typbyval == (typlen > 0 && typlen <= 8). Something to
think about.
Hi, On 2025-08-12 08:30:43 +0200, Peter Eisentraut wrote: > I'm also thinking, as a follow-on project, we could get rid of typbyval and > require that typbyval == (typlen > 0 && typlen <= 8). Something to think > about. We currently have types that aren't typbyval despite fitting those criteria: postgres[1606972][1]=# SELECT oid::regtype, typlen FROM pg_type WHERE typlen > 0 and typlen <= 8 and not typbyval; ┌──────────┬────────┐ │ oid │ typlen │ ├──────────┼────────┤ │ tid │ 6 │ │ macaddr │ 6 │ │ macaddr8 │ 8 │ └──────────┴────────┘ (3 rows) Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2025-08-12 08:30:43 +0200, Peter Eisentraut wrote:
>> I'm also thinking, as a follow-on project, we could get rid of typbyval and
>> require that typbyval == (typlen > 0 && typlen <= 8). Something to think
>> about.
> We currently have types that aren't typbyval despite fitting those criteria:
We could change those three if we had a mind to. Changing TID would
probably risk some bugs, but we pass it around enough in UPDATE/DELETE
that making it typbyval would surely buy some small performance win.
(I doubt anyone would notice about the macaddr types though.)
However, I'm really hesitant to start enforcing such a thing against
user-defined types. I think that would translate to breaking some
extensions in return for not much.
regards, tom lane
Peter Eisentraut <peter@eisentraut.org> writes:
> These patches look mechanically correct to me.
Thanks for reviewing!
> The various additional uses of *GetDatum and DatumGet* could be applied
> as a separate patch. That would make the remaining patch clearer as
> mostly just removing dead code.
I looked at doing this, but couldn't convince myself that it would be
anything but make-work. It's not like 0002 and 0003 could be pure
deletions in any case -- at least there would be comment updates to
make.
> In tupmacs.h, I think the two sites with
> case sizeof(Datum):
> should be rewritten using
> case sizeof(int64):
> to match the other cases. Otherwise, this code ends up looking
> mysteriously asymmetric. (And the related code in pg_type.c as well.)
Fair point, and I also realized that these were another place to
insert Datum conversions :-(. The existing code would fail badly
if we redefined Datum as a struct with an extra field in it.
> The remaining mentions of "SIZEOF_DATUM" in gistproc.c and network.c
> could be replaced by "sizeof(Datum)". Then we could eventually remove
> SIZEOF_DATUM altogether.
Agreed with s/SIZEOF_DATUM/sizeof(Datum)/g, so I've done that. I do
not think removing the #define altogether will be sensible, however.
It will accomplish little except to break extensions that we didn't
need to break. It'd be particularly painful for extensions that
want to compile the same source code against multiple PG versions.
However, it does make sense to put in a comment explaining that
SIZEOF_DATUM is now vestigial, so I've done that.
> (Maybe DatumBigEndianToNative() would better
> live in a different header file at that point, not sure.)
Not sure either. I did realize that #ifdef'ing it with SIZEOF_DATUM
was pretty pointless, so I undid that aspect.
> I would also remove most of the remaining uses of FLOAT8PASSBYVAL,
> especially where it is used in relation with INT8OID, which is just
> endlessly confusing.
Fair enough, also done below.
> We should also get rid of these things in the control file and the ABI
> magic structs, but that could be done as separate patches. It would
> probably require a separate round of thinking about compatibility,
> upgrading, etc.
Meh. The savings is not enough to justify expending any brain cells
working through the implications. There would be implications, too,
for example with programs that look at the output of pg_controldata.
Again, just labeling those fields as vestigial seems like enough.
PFA v2 with these changes. I feel like this is pretty close to
committable now.
regards, tom lane
From 594400bcdd9637a1f77f22ef0664cce7046ef1f5 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 12 Aug 2025 14:53:35 -0400
Subject: [PATCH v2 1/3] Make type Datum be 8 bytes wide everywhere.
This patch makes sizeof(Datum) be 8 on all platforms including
32-bit ones. The objective is to allow USE_FLOAT8_BYVAL to be true
everywhere, and in consequence to remove a lot of code that is
specific to pass-by-reference handling of float8, int8, etc. In this
way we can reduce the maintenance effort involved in supporting 32-bit
platforms, without going so far as to actually desupport them. Since
Datum is strictly an in-memory concept, this has no impact on on-disk
storage, though an initdb or pg_upgrade will be needed to fix affected
pg_type.typbyval entries (and also Const nodes in stored queries).
We have required platforms to support [u]int64 for ages, so this
breaks no supported platform. We can expect that this change will
make 32-bit builds a bit slower and more memory-hungry, although being
able to use pass-by-value handling of 8-byte types may buy back some
of that. But we stopped optimizing for 32-bit cases a long time ago,
and this seems like just another step on that path.
This initial patch simply forces the correct type definition and
USE_FLOAT8_BYVAL setting, and cleans up a couple of minor compiler
complaints that ensued. This is sufficient for testing purposes.
In the wake of a bunch of Datum-conversion cleanups by Peter
Eisentraut, this now compiles cleanly with gcc on a 32-bit platform.
(I'd only tested the previous version with clang, which it turns out
is less picky than gcc about width-changing coercions.)
There is a good deal of now-dead code that I'll remove in separate
follow-up patches.
XXX don't forget catversion bump.
Discussion: https://postgr.es/m/1749799.1752797397@sss.pgh.pa.us
---
src/backend/storage/ipc/ipc.c | 2 +-
src/backend/utils/resowner/resowner.c | 7 ++-----
src/include/nodes/nodes.h | 8 ++++++--
src/include/pg_config_manual.h | 13 ++++---------
src/include/postgres.h | 21 +++++++++++++--------
5 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index 567739b5be9..2704e80b3a7 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -399,7 +399,7 @@ cancel_before_shmem_exit(pg_on_exit_callback function, Datum arg)
before_shmem_exit_list[before_shmem_exit_index - 1].arg == arg)
--before_shmem_exit_index;
else
- elog(ERROR, "before_shmem_exit callback (%p,0x%" PRIxPTR ") is not the latest entry",
+ elog(ERROR, "before_shmem_exit callback (%p,0x%" PRIx64 ") is not the latest entry",
function, arg);
}
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index d39f3e1b655..fca84ded6dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -231,11 +231,8 @@ hash_resource_elem(Datum value, const ResourceOwnerDesc *kind)
* 'kind' into the hash. Just add it with hash_combine(), it perturbs the
* result enough for our purposes.
*/
-#if SIZEOF_DATUM == 8
- return hash_combine64(murmurhash64((uint64) value), (uint64) kind);
-#else
- return hash_combine(murmurhash32((uint32) value), (uint32) kind);
-#endif
+ return hash_combine64(murmurhash64((uint64) value),
+ (uint64) (uintptr_t) kind);
}
/*
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index fbe333d88fa..b2dc380b57b 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -188,6 +188,8 @@ castNodeImpl(NodeTag type, void *ptr)
* ----------------------------------------------------------------
*/
+#ifndef FRONTEND
+
/*
* nodes/{outfuncs.c,print.c}
*/
@@ -198,7 +200,7 @@ extern void outNode(struct StringInfoData *str, const void *obj);
extern void outToken(struct StringInfoData *str, const char *s);
extern void outBitmapset(struct StringInfoData *str,
const struct Bitmapset *bms);
-extern void outDatum(struct StringInfoData *str, uintptr_t value,
+extern void outDatum(struct StringInfoData *str, Datum value,
int typlen, bool typbyval);
extern char *nodeToString(const void *obj);
extern char *nodeToStringWithLocations(const void *obj);
@@ -212,7 +214,7 @@ extern void *stringToNode(const char *str);
extern void *stringToNodeWithLocations(const char *str);
#endif
extern struct Bitmapset *readBitmapset(void);
-extern uintptr_t readDatum(bool typbyval);
+extern Datum readDatum(bool typbyval);
extern bool *readBoolCols(int numCols);
extern int *readIntCols(int numCols);
extern Oid *readOidCols(int numCols);
@@ -235,6 +237,8 @@ extern void *copyObjectImpl(const void *from);
*/
extern bool equal(const void *a, const void *b);
+#endif /* !FRONTEND */
+
/*
* Typedef for parse location. This is just an int, but this way
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 125d3eb5fff..7e1aa422332 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -74,17 +74,12 @@
#define PARTITION_MAX_KEYS 32
/*
- * Decide whether built-in 8-byte types, including float8, int8, and
- * timestamp, are passed by value. This is on by default if sizeof(Datum) >=
- * 8 (that is, on 64-bit platforms). If sizeof(Datum) < 8 (32-bit platforms),
- * this must be off. We keep this here as an option so that it is easy to
- * test the pass-by-reference code paths on 64-bit platforms.
- *
- * Changing this requires an initdb.
+ * This symbol is now vestigial: built-in 8-byte types, including float8,
+ * int8, and timestamp, are always passed by value since we require Datum
+ * to be wide enough to permit that. We continue to define the symbol here
+ * so as not to unnecessarily break extension code.
*/
-#if SIZEOF_VOID_P >= 8
#define USE_FLOAT8_BYVAL 1
-#endif
/*
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 8a41a668687..e81829bfa6f 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -58,15 +58,22 @@
/*
* A Datum contains either a value of a pass-by-value type or a pointer to a
- * value of a pass-by-reference type. Therefore, we require:
- *
- * sizeof(Datum) == sizeof(void *) == 4 or 8
+ * value of a pass-by-reference type. Therefore, we must have
+ * sizeof(Datum) >= sizeof(void *). No current or foreseeable Postgres
+ * platform has pointers wider than 8 bytes, and standardizing on Datum being
+ * exactly 8 bytes has advantages in reducing cross-platform differences.
*
* The functions below and the analogous functions for other types should be used to
* convert between a Datum and the appropriate C type.
*/
-typedef uintptr_t Datum;
+typedef uint64_t Datum;
+
+/*
+ * This symbol is now vestigial, but we continue to define it so as not to
+ * unnecessarily break extension code.
+ */
+#define SIZEOF_DATUM 8
/*
* A NullableDatum is used in places where both a Datum and its nullness needs
@@ -83,8 +90,6 @@ typedef struct NullableDatum
/* due to alignment padding this could be used for flags for free */
} NullableDatum;
-#define SIZEOF_DATUM SIZEOF_VOID_P
-
/*
* DatumGetBool
* Returns boolean value of a datum.
@@ -316,7 +321,7 @@ CommandIdGetDatum(CommandId X)
static inline Pointer
DatumGetPointer(Datum X)
{
- return (Pointer) X;
+ return (Pointer) (uintptr_t) X;
}
/*
@@ -326,7 +331,7 @@ DatumGetPointer(Datum X)
static inline Datum
PointerGetDatum(const void *X)
{
- return (Datum) X;
+ return (Datum) (uintptr_t) X;
}
/*
--
2.43.7
From 7f4df2956c3e7940efec879237f8b79e8d8a76bd Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 12 Aug 2025 15:13:23 -0400
Subject: [PATCH v2 2/3] Grab the low-hanging fruit from forcing sizeof(Datum)
to 8.
Remove conditionally-compiled code for smaller Datum widths.
I also fixed up a few more places that were not using
DatumGetIntXX where they should, and made some cosmetic
adjustments such as using sizeof(int64) not sizeof(Datum)
in places where that fit better with the surrounding code.
One thing I remembered while preparing this part is that SP-GiST
stores pass-by-value prefix keys as Datums, so that the on-disk
representation depends on sizeof(Datum). That's even more
unfortunate than the existing commentary makes it out to be,
because now there is a hazard that the change of sizeof(Datum)
will break SP-GiST indexes on 32-bit machines. It appears that
there are no existing SP-GiST opclasses that are actually
affected; and if there are some that I didn't find, the number
of installations that are using them on 32-bit machines is
doubtless tiny. So I'm proceeding on the assumption that we
can get away with this, but it's something to worry about.
(gininsert.c looks like it has a similar problem, but it's okay
because the "tuples" it's constructing are just transient data
within the tuplesort step. That's pretty poorly documented
though, so I added some comments.)
Discussion: https://postgr.es/m/1749799.1752797397@sss.pgh.pa.us
---
doc/src/sgml/xfunc.sgml | 3 +-
src/backend/access/gin/gininsert.c | 5 +-
src/backend/access/gist/gistproc.c | 10 +--
src/backend/access/nbtree/nbtcompare.c | 20 -----
src/backend/catalog/pg_type.c | 4 +-
src/backend/utils/adt/mac.c | 27 +++----
src/backend/utils/adt/network.c | 14 +---
src/backend/utils/adt/numeric.c | 106 ++-----------------------
src/backend/utils/adt/timestamp.c | 21 -----
src/backend/utils/adt/uuid.c | 6 +-
src/backend/utils/adt/varlena.c | 12 +--
src/backend/utils/sort/tuplesort.c | 8 --
src/include/access/gin_tuple.h | 4 +-
src/include/access/spgist_private.h | 10 ++-
src/include/access/tupmacs.h | 12 +--
src/include/port/pg_bswap.h | 17 +---
src/include/utils/sortsupport.h | 4 -
17 files changed, 50 insertions(+), 233 deletions(-)
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 30219f432d9..f116d0648e5 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -2051,8 +2051,7 @@ PG_MODULE_MAGIC_EXT(
</para>
<para>
- By-value types can only be 1, 2, or 4 bytes in length
- (also 8 bytes, if <literal>sizeof(Datum)</literal> is 8 on your machine).
+ By-value types can only be 1, 2, 4, or 8 bytes in length.
You should be careful to define your types such that they will be the
same size (in bytes) on all architectures. For example, the
<literal>long</literal> type is dangerous because it is 4 bytes on some
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 47b1898a064..e9d4b27427e 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -2189,7 +2189,10 @@ typedef struct
* we simply copy the whole Datum, so that we don't have to care about stuff
* like endianess etc. We could make it a little bit smaller, but it's not
* worth it - it's a tiny fraction of the data, and we need to MAXALIGN the
- * start of the TID list anyway. So we wouldn't save anything.
+ * start of the TID list anyway. So we wouldn't save anything. (This would
+ * not be a good idea for the permanent in-index data, since we'd prefer
+ * that that not depend on sizeof(Datum). But this is just a transient
+ * representation to use while sorting the data.)
*
* The TID list is serialized as compressed - it's highly compressible, and
* we already have ginCompressPostingList for this purpose. The list may be
diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c
index 392163cb229..f2ec6cbe2e5 100644
--- a/src/backend/access/gist/gistproc.c
+++ b/src/backend/access/gist/gistproc.c
@@ -1707,8 +1707,8 @@ gist_bbox_zorder_cmp(Datum a, Datum b, SortSupport ssup)
* Abbreviated version of Z-order comparison
*
* The abbreviated format is a Z-order value computed from the two 32-bit
- * floats. If SIZEOF_DATUM == 8, the 64-bit Z-order value fits fully in the
- * abbreviated Datum, otherwise use its most significant bits.
+ * floats. Now that sizeof(Datum) is always 8, the 64-bit Z-order value
+ * always fits fully in the abbreviated Datum.
*/
static Datum
gist_bbox_zorder_abbrev_convert(Datum original, SortSupport ssup)
@@ -1718,11 +1718,7 @@ gist_bbox_zorder_abbrev_convert(Datum original, SortSupport ssup)
z = point_zorder_internal(p->x, p->y);
-#if SIZEOF_DATUM == 8
- return (Datum) z;
-#else
- return (Datum) (z >> 32);
-#endif
+ return UInt64GetDatum(z);
}
/*
diff --git a/src/backend/access/nbtree/nbtcompare.c b/src/backend/access/nbtree/nbtcompare.c
index e1b52acd20d..188c27b4925 100644
--- a/src/backend/access/nbtree/nbtcompare.c
+++ b/src/backend/access/nbtree/nbtcompare.c
@@ -278,32 +278,12 @@ btint8cmp(PG_FUNCTION_ARGS)
PG_RETURN_INT32(A_LESS_THAN_B);
}
-#if SIZEOF_DATUM < 8
-static int
-btint8fastcmp(Datum x, Datum y, SortSupport ssup)
-{
- int64 a = DatumGetInt64(x);
- int64 b = DatumGetInt64(y);
-
- if (a > b)
- return A_GREATER_THAN_B;
- else if (a == b)
- return 0;
- else
- return A_LESS_THAN_B;
-}
-#endif
-
Datum
btint8sortsupport(PG_FUNCTION_ARGS)
{
SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
-#if SIZEOF_DATUM >= 8
ssup->comparator = ssup_datum_signed_cmp;
-#else
- ssup->comparator = btint8fastcmp;
-#endif
PG_RETURN_VOID();
}
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 1ec523ee3e5..3cd9b69edc5 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -285,8 +285,7 @@ TypeCreate(Oid newTypeOid,
errmsg("alignment \"%c\" is invalid for passed-by-value type of size %d",
alignment, internalSize)));
}
-#if SIZEOF_DATUM == 8
- else if (internalSize == (int16) sizeof(Datum))
+ else if (internalSize == (int16) sizeof(int64))
{
if (alignment != TYPALIGN_DOUBLE)
ereport(ERROR,
@@ -294,7 +293,6 @@ TypeCreate(Oid newTypeOid,
errmsg("alignment \"%c\" is invalid for passed-by-value type of size %d",
alignment, internalSize)));
}
-#endif
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
diff --git a/src/backend/utils/adt/mac.c b/src/backend/utils/adt/mac.c
index 3644e9735f5..bb38ef2f5e4 100644
--- a/src/backend/utils/adt/mac.c
+++ b/src/backend/utils/adt/mac.c
@@ -481,33 +481,26 @@ macaddr_abbrev_convert(Datum original, SortSupport ssup)
Datum res;
/*
- * On a 64-bit machine, zero out the 8-byte datum and copy the 6 bytes of
- * the MAC address in. There will be two bytes of zero padding on the end
- * of the least significant bits.
+ * Zero out the 8-byte Datum and copy in the 6 bytes of the MAC address.
+ * There will be two bytes of zero padding on the end of the least
+ * significant bits.
*/
-#if SIZEOF_DATUM == 8
- memset(&res, 0, SIZEOF_DATUM);
+ StaticAssertStmt(sizeof(res) >= sizeof(macaddr),
+ "Datum is too small for macaddr");
+ memset(&res, 0, sizeof(res));
memcpy(&res, authoritative, sizeof(macaddr));
-#else /* SIZEOF_DATUM != 8 */
- memcpy(&res, authoritative, SIZEOF_DATUM);
-#endif
uss->input_count += 1;
/*
- * Cardinality estimation. The estimate uses uint32, so on a 64-bit
- * architecture, XOR the two 32-bit halves together to produce slightly
- * more entropy. The two zeroed bytes won't have any practical impact on
- * this operation.
+ * Cardinality estimation. The estimate uses uint32, so XOR the two 32-bit
+ * halves together to produce slightly more entropy. The two zeroed bytes
+ * won't have any practical impact on this operation.
*/
if (uss->estimating)
{
uint32 tmp;
-#if SIZEOF_DATUM == 8
- tmp = (uint32) res ^ (uint32) ((uint64) res >> 32);
-#else /* SIZEOF_DATUM != 8 */
- tmp = (uint32) res;
-#endif
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
}
diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c
index 9fd211b2d45..6e1ffee0655 100644
--- a/src/backend/utils/adt/network.c
+++ b/src/backend/utils/adt/network.c
@@ -659,7 +659,7 @@ network_abbrev_convert(Datum original, SortSupport ssup)
ipaddr_datum = DatumBigEndianToNative(ipaddr_datum);
/* Initialize result with ipfamily (most significant) bit set */
- res = ((Datum) 1) << (SIZEOF_DATUM * BITS_PER_BYTE - 1);
+ res = ((Datum) 1) << (sizeof(Datum) * BITS_PER_BYTE - 1);
}
/*
@@ -681,14 +681,14 @@ network_abbrev_convert(Datum original, SortSupport ssup)
subnet_size = ip_maxbits(authoritative) - ip_bits(authoritative);
Assert(subnet_size >= 0);
/* subnet size must work with prefix ipaddr cases */
- subnet_size %= SIZEOF_DATUM * BITS_PER_BYTE;
+ subnet_size %= sizeof(Datum) * BITS_PER_BYTE;
if (ip_bits(authoritative) == 0)
{
/* Fit as many ipaddr bits as possible into subnet */
subnet_bitmask = ((Datum) 0) - 1;
network = 0;
}
- else if (ip_bits(authoritative) < SIZEOF_DATUM * BITS_PER_BYTE)
+ else if (ip_bits(authoritative) < sizeof(Datum) * BITS_PER_BYTE)
{
/* Split ipaddr bits between network and subnet */
subnet_bitmask = (((Datum) 1) << subnet_size) - 1;
@@ -701,7 +701,6 @@ network_abbrev_convert(Datum original, SortSupport ssup)
network = ipaddr_datum;
}
-#if SIZEOF_DATUM == 8
if (ip_family(authoritative) == PGSQL_AF_INET)
{
/*
@@ -750,7 +749,6 @@ network_abbrev_convert(Datum original, SortSupport ssup)
res |= network | netmask_size | subnet;
}
else
-#endif
{
/*
* 4 byte datums, or IPv6 with 8 byte datums: Use as many of the
@@ -767,11 +765,7 @@ network_abbrev_convert(Datum original, SortSupport ssup)
{
uint32 tmp;
-#if SIZEOF_DATUM == 8
- tmp = (uint32) res ^ (uint32) ((uint64) res >> 32);
-#else /* SIZEOF_DATUM != 8 */
- tmp = (uint32) res;
-#endif
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 122f2efab8b..4b94ee9a3b5 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -392,30 +392,21 @@ typedef struct NumericSumAccum
/*
* We define our own macros for packing and unpacking abbreviated-key
- * representations for numeric values in order to avoid depending on
- * USE_FLOAT8_BYVAL. The type of abbreviation we use is based only on
- * the size of a datum, not the argument-passing convention for float8.
+ * representations, just to have a notational indication that that's
+ * what we're doing. Now that sizeof(Datum) is always 8, we can rely
+ * on fitting an int64 into Datum.
*
- * The range of abbreviations for finite values is from +PG_INT64/32_MAX
- * to -PG_INT64/32_MAX. NaN has the abbreviation PG_INT64/32_MIN, and we
+ * The range of abbreviations for finite values is from +PG_INT64_MAX
+ * to -PG_INT64_MAX. NaN has the abbreviation PG_INT64_MIN, and we
* define the sort ordering to make that work out properly (see further
* comments below). PINF and NINF share the abbreviations of the largest
* and smallest finite abbreviation classes.
*/
-#define NUMERIC_ABBREV_BITS (SIZEOF_DATUM * BITS_PER_BYTE)
-#if SIZEOF_DATUM == 8
-#define NumericAbbrevGetDatum(X) ((Datum) (X))
-#define DatumGetNumericAbbrev(X) ((int64) (X))
+#define NumericAbbrevGetDatum(X) Int64GetDatum(X)
+#define DatumGetNumericAbbrev(X) DatumGetInt64(X)
#define NUMERIC_ABBREV_NAN NumericAbbrevGetDatum(PG_INT64_MIN)
#define NUMERIC_ABBREV_PINF NumericAbbrevGetDatum(-PG_INT64_MAX)
#define NUMERIC_ABBREV_NINF NumericAbbrevGetDatum(PG_INT64_MAX)
-#else
-#define NumericAbbrevGetDatum(X) ((Datum) (X))
-#define DatumGetNumericAbbrev(X) ((int32) (X))
-#define NUMERIC_ABBREV_NAN NumericAbbrevGetDatum(PG_INT32_MIN)
-#define NUMERIC_ABBREV_PINF NumericAbbrevGetDatum(-PG_INT32_MAX)
-#define NUMERIC_ABBREV_NINF NumericAbbrevGetDatum(PG_INT32_MAX)
-#endif
/* ----------
@@ -2328,7 +2319,7 @@ numeric_cmp_abbrev(Datum x, Datum y, SortSupport ssup)
}
/*
- * Abbreviate a NumericVar according to the available bit size.
+ * Abbreviate a NumericVar into the 64-bit sortsupport size.
*
* The 31-bit value is constructed as:
*
@@ -2372,9 +2363,6 @@ numeric_cmp_abbrev(Datum x, Datum y, SortSupport ssup)
* with all bits zero. This allows simple comparisons to work on the composite
* value.
*/
-
-#if NUMERIC_ABBREV_BITS == 64
-
static Datum
numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss)
{
@@ -2426,84 +2414,6 @@ numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss)
return NumericAbbrevGetDatum(result);
}
-#endif /* NUMERIC_ABBREV_BITS == 64 */
-
-#if NUMERIC_ABBREV_BITS == 32
-
-static Datum
-numeric_abbrev_convert_var(const NumericVar *var, NumericSortSupport *nss)
-{
- int ndigits = var->ndigits;
- int weight = var->weight;
- int32 result;
-
- if (ndigits == 0 || weight < -11)
- {
- result = 0;
- }
- else if (weight > 20)
- {
- result = PG_INT32_MAX;
- }
- else
- {
- NumericDigit nxt1 = (ndigits > 1) ? var->digits[1] : 0;
-
- weight = (weight + 11) * 4;
-
- result = var->digits[0];
-
- /*
- * "result" now has 1 to 4 nonzero decimal digits. We pack in more
- * digits to make 7 in total (largest we can fit in 24 bits)
- */
-
- if (result > 999)
- {
- /* already have 4 digits, add 3 more */
- result = (result * 1000) + (nxt1 / 10);
- weight += 3;
- }
- else if (result > 99)
- {
- /* already have 3 digits, add 4 more */
- result = (result * 10000) + nxt1;
- weight += 2;
- }
- else if (result > 9)
- {
- NumericDigit nxt2 = (ndigits > 2) ? var->digits[2] : 0;
-
- /* already have 2 digits, add 5 more */
- result = (result * 100000) + (nxt1 * 10) + (nxt2 / 1000);
- weight += 1;
- }
- else
- {
- NumericDigit nxt2 = (ndigits > 2) ? var->digits[2] : 0;
-
- /* already have 1 digit, add 6 more */
- result = (result * 1000000) + (nxt1 * 100) + (nxt2 / 100);
- }
-
- result = result | (weight << 24);
- }
-
- /* the abbrev is negated relative to the original */
- if (var->sign == NUMERIC_POS)
- result = -result;
-
- if (nss->estimating)
- {
- uint32 tmp = (uint32) result;
-
- addHyperLogLog(&nss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
- }
-
- return NumericAbbrevGetDatum(result);
-}
-
-#endif /* NUMERIC_ABBREV_BITS == 32 */
/*
* Ordinary (non-sortsupport) comparisons follow.
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index e640b48205b..3e5f9dc1458 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -2275,33 +2275,12 @@ timestamp_cmp(PG_FUNCTION_ARGS)
PG_RETURN_INT32(timestamp_cmp_internal(dt1, dt2));
}
-#if SIZEOF_DATUM < 8
-/* note: this is used for timestamptz also */
-static int
-timestamp_fastcmp(Datum x, Datum y, SortSupport ssup)
-{
- Timestamp a = DatumGetTimestamp(x);
- Timestamp b = DatumGetTimestamp(y);
-
- return timestamp_cmp_internal(a, b);
-}
-#endif
-
Datum
timestamp_sortsupport(PG_FUNCTION_ARGS)
{
SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
-#if SIZEOF_DATUM >= 8
-
- /*
- * If this build has pass-by-value timestamps, then we can use a standard
- * comparator function.
- */
ssup->comparator = ssup_datum_signed_cmp;
-#else
- ssup->comparator = timestamp_fastcmp;
-#endif
PG_RETURN_VOID();
}
diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index bce7309c183..7413239f7af 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -398,11 +398,7 @@ uuid_abbrev_convert(Datum original, SortSupport ssup)
{
uint32 tmp;
-#if SIZEOF_DATUM == 8
- tmp = (uint32) res ^ (uint32) ((uint64) res >> 32);
-#else /* SIZEOF_DATUM != 8 */
- tmp = (uint32) res;
-#endif
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
addHyperLogLog(&uss->abbr_card, DatumGetUInt32(hash_uint32(tmp)));
}
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 11b442a5941..4fc3f0886a4 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2132,18 +2132,12 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
addHyperLogLog(&sss->full_card, hash);
/* Hash abbreviated key */
-#if SIZEOF_DATUM == 8
{
- uint32 lohalf,
- hihalf;
+ uint32 tmp;
- lohalf = (uint32) res;
- hihalf = (uint32) (res >> 32);
- hash = DatumGetUInt32(hash_uint32(lohalf ^ hihalf));
+ tmp = DatumGetUInt32(res) ^ (uint32) (DatumGetUInt64(res) >> 32);
+ hash = DatumGetUInt32(hash_uint32(tmp));
}
-#else /* SIZEOF_DATUM != 8 */
- hash = DatumGetUInt32(hash_uint32((uint32) res));
-#endif
addHyperLogLog(&sss->abbr_card, hash);
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 65ab83fff8b..5d4411dc33f 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -512,7 +512,6 @@ qsort_tuple_unsigned_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
return state->base.comparetup_tiebreak(a, b, state);
}
-#if SIZEOF_DATUM >= 8
/* Used if first key's comparator is ssup_datum_signed_cmp */
static pg_attribute_always_inline int
qsort_tuple_signed_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
@@ -535,7 +534,6 @@ qsort_tuple_signed_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
return state->base.comparetup_tiebreak(a, b, state);
}
-#endif
/* Used if first key's comparator is ssup_datum_int32_cmp */
static pg_attribute_always_inline int
@@ -578,7 +576,6 @@ qsort_tuple_int32_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
#define ST_DEFINE
#include "lib/sort_template.h"
-#if SIZEOF_DATUM >= 8
#define ST_SORT qsort_tuple_signed
#define ST_ELEMENT_TYPE SortTuple
#define ST_COMPARE(a, b, state) qsort_tuple_signed_compare(a, b, state)
@@ -587,7 +584,6 @@ qsort_tuple_int32_compare(SortTuple *a, SortTuple *b, Tuplesortstate *state)
#define ST_SCOPE static
#define ST_DEFINE
#include "lib/sort_template.h"
-#endif
#define ST_SORT qsort_tuple_int32
#define ST_ELEMENT_TYPE SortTuple
@@ -2692,7 +2688,6 @@ tuplesort_sort_memtuples(Tuplesortstate *state)
state);
return;
}
-#if SIZEOF_DATUM >= 8
else if (state->base.sortKeys[0].comparator == ssup_datum_signed_cmp)
{
qsort_tuple_signed(state->memtuples,
@@ -2700,7 +2695,6 @@ tuplesort_sort_memtuples(Tuplesortstate *state)
state);
return;
}
-#endif
else if (state->base.sortKeys[0].comparator == ssup_datum_int32_cmp)
{
qsort_tuple_int32(state->memtuples,
@@ -3146,7 +3140,6 @@ ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup)
return 0;
}
-#if SIZEOF_DATUM >= 8
int
ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup)
{
@@ -3160,7 +3153,6 @@ ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup)
else
return 0;
}
-#endif
int
ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup)
diff --git a/src/include/access/gin_tuple.h b/src/include/access/gin_tuple.h
index 702f7d12889..b4f103dec9a 100644
--- a/src/include/access/gin_tuple.h
+++ b/src/include/access/gin_tuple.h
@@ -15,7 +15,9 @@
#include "utils/sortsupport.h"
/*
- * Data for one key in a GIN index.
+ * Data for one key in a GIN index. (This is not the permanent in-index
+ * representation, but just a convenient format to use during the tuplesort
+ * stage of building a new GIN index.)
*/
typedef struct GinTuple
{
diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h
index cb43a278f46..fdda7f4c639 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -285,10 +285,12 @@ typedef struct SpGistCache
* If the prefix datum is of a pass-by-value type, it is stored in its
* Datum representation, that is its on-disk representation is of length
* sizeof(Datum). This is a fairly unfortunate choice, because in no other
- * place does Postgres use Datum as an on-disk representation; it creates
- * an unnecessary incompatibility between 32-bit and 64-bit builds. But the
- * compatibility loss is mostly theoretical since MAXIMUM_ALIGNOF typically
- * differs between such builds, too. Anyway we're stuck with it now.
+ * place does Postgres use Datum as an on-disk representation. Formerly it
+ * meant an unnecessary incompatibility between 32-bit and 64-bit builds, and
+ * as of v19 it instead creates a hazard for binary upgrades on 32-bit builds.
+ * Fortunately, that hazard seems mostly theoretical for lack of affected
+ * opclasses. Going forward, we will be using a fixed size of Datum so that
+ * there's no longer any pressing reason to change this.
*/
typedef struct SpGistInnerTupleData
{
diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h
index 6240ec930e7..3a05bf7ca66 100644
--- a/src/include/access/tupmacs.h
+++ b/src/include/access/tupmacs.h
@@ -62,10 +62,8 @@ fetch_att(const void *T, bool attbyval, int attlen)
return Int16GetDatum(*((const int16 *) T));
case sizeof(int32):
return Int32GetDatum(*((const int32 *) T));
-#if SIZEOF_DATUM == 8
- case sizeof(Datum):
- return *((const Datum *) T);
-#endif
+ case sizeof(int64):
+ return Int64GetDatum(*((const int64 *) T));
default:
elog(ERROR, "unsupported byval length: %d", attlen);
return 0;
@@ -221,11 +219,9 @@ store_att_byval(void *T, Datum newdatum, int attlen)
case sizeof(int32):
*(int32 *) T = DatumGetInt32(newdatum);
break;
-#if SIZEOF_DATUM == 8
- case sizeof(Datum):
- *(Datum *) T = newdatum;
+ case sizeof(int64):
+ *(int64 *) T = DatumGetInt64(newdatum);
break;
-#endif
default:
elog(ERROR, "unsupported byval length: %d", attlen);
}
diff --git a/src/include/port/pg_bswap.h b/src/include/port/pg_bswap.h
index 33648433c63..b15f6f6ac38 100644
--- a/src/include/port/pg_bswap.h
+++ b/src/include/port/pg_bswap.h
@@ -130,8 +130,7 @@ pg_bswap64(uint64 x)
/*
* Rearrange the bytes of a Datum from big-endian order into the native byte
- * order. On big-endian machines, this does nothing at all. Note that the C
- * type Datum is an unsigned integer type on all platforms.
+ * order. On big-endian machines, this does nothing at all.
*
* One possible application of the DatumBigEndianToNative() macro is to make
* bitwise comparisons cheaper. A simple 3-way comparison of Datums
@@ -139,23 +138,11 @@ pg_bswap64(uint64 x)
* the same result as a memcmp() of the corresponding original Datums, but can
* be much cheaper. It's generally safe to do this on big-endian systems
* without any special transformation occurring first.
- *
- * If SIZEOF_DATUM is not defined, then postgres.h wasn't included and these
- * macros probably shouldn't be used, so we define nothing. Note that
- * SIZEOF_DATUM == 8 would evaluate as 0 == 8 in that case, potentially
- * leading to the wrong implementation being selected and confusing errors, so
- * defining nothing is safest.
*/
-#ifdef SIZEOF_DATUM
#ifdef WORDS_BIGENDIAN
#define DatumBigEndianToNative(x) (x)
#else /* !WORDS_BIGENDIAN */
-#if SIZEOF_DATUM == 8
-#define DatumBigEndianToNative(x) pg_bswap64(x)
-#else /* SIZEOF_DATUM != 8 */
-#define DatumBigEndianToNative(x) pg_bswap32(x)
-#endif /* SIZEOF_DATUM == 8 */
+#define DatumBigEndianToNative(x) UInt64GetDatum(pg_bswap64(DatumGetUInt64(x)))
#endif /* WORDS_BIGENDIAN */
-#endif /* SIZEOF_DATUM */
#endif /* PG_BSWAP_H */
diff --git a/src/include/utils/sortsupport.h b/src/include/utils/sortsupport.h
index b7abaf7802d..c64527e2ee9 100644
--- a/src/include/utils/sortsupport.h
+++ b/src/include/utils/sortsupport.h
@@ -262,7 +262,6 @@ ApplyUnsignedSortComparator(Datum datum1, bool isNull1,
return compare;
}
-#if SIZEOF_DATUM >= 8
static inline int
ApplySignedSortComparator(Datum datum1, bool isNull1,
Datum datum2, bool isNull2,
@@ -296,7 +295,6 @@ ApplySignedSortComparator(Datum datum1, bool isNull1,
return compare;
}
-#endif
static inline int
ApplyInt32SortComparator(Datum datum1, bool isNull1,
@@ -376,9 +374,7 @@ ApplySortAbbrevFullComparator(Datum datum1, bool isNull1,
* are eligible for faster sorting.
*/
extern int ssup_datum_unsigned_cmp(Datum x, Datum y, SortSupport ssup);
-#if SIZEOF_DATUM >= 8
extern int ssup_datum_signed_cmp(Datum x, Datum y, SortSupport ssup);
-#endif
extern int ssup_datum_int32_cmp(Datum x, Datum y, SortSupport ssup);
/* Other functions in utils/sort/sortsupport.c */
--
2.43.7
From 637699159844a583fd96fbe3d9008d36743289f0 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 12 Aug 2025 15:32:45 -0400
Subject: [PATCH v2 3/3] Grab the low-hanging fruit from forcing
USE_FLOAT8_BYVAL to true.
Remove conditionally-compiled code for the other case.
Replace uses of FLOAT8PASSBYVAL with constant "true", mainly because
it was quite confusing in cases where the type we were dealing with
wasn't float8.
I left the associated pg_control and Pg_magic_struct fields in place.
Perhaps we should get rid of them, but it would save little, so it
doesn't seem worth thinking hard about the compatibility implications.
I just labeled them "vestigial" in places where that seemed helpful.
Discussion: https://postgr.es/m/1749799.1752797397@sss.pgh.pa.us
---
contrib/btree_gist/btree_time.c | 51 ++++++-------
contrib/btree_gist/btree_ts.c | 39 ++++------
src/backend/access/common/tupdesc.c | 2 +-
src/backend/access/index/indexam.c | 5 --
src/backend/access/transam/xlog.c | 20 +----
src/backend/catalog/genbki.pl | 3 +-
src/backend/optimizer/plan/planagg.c | 2 +-
src/backend/optimizer/plan/planner.c | 4 +-
src/backend/parser/parse_node.c | 2 +-
src/backend/rewrite/rewriteSearchCycle.c | 2 +-
src/backend/utils/adt/arrayfuncs.c | 6 +-
src/backend/utils/adt/int8.c | 75 ++++---------------
src/backend/utils/adt/numeric.c | 74 ++++--------------
src/backend/utils/adt/orderedsetaggs.c | 2 +-
src/backend/utils/adt/rangetypes_typanalyze.c | 2 +-
src/backend/utils/fmgr/fmgr.c | 35 ---------
src/bin/initdb/initdb.c | 3 -
src/bin/pg_resetwal/pg_resetwal.c | 2 +-
src/include/c.h | 8 +-
src/include/catalog/pg_type.dat | 16 ++--
src/include/fmgr.h | 2 +-
src/include/postgres.h | 58 +-------------
22 files changed, 100 insertions(+), 313 deletions(-)
diff --git a/contrib/btree_gist/btree_time.c b/contrib/btree_gist/btree_time.c
index 1dba95057ba..fd6a2e05bc7 100644
--- a/contrib/btree_gist/btree_time.c
+++ b/contrib/btree_gist/btree_time.c
@@ -31,13 +31,6 @@ PG_FUNCTION_INFO_V1(gbt_time_sortsupport);
PG_FUNCTION_INFO_V1(gbt_timetz_sortsupport);
-#ifdef USE_FLOAT8_BYVAL
-#define TimeADTGetDatumFast(X) TimeADTGetDatum(X)
-#else
-#define TimeADTGetDatumFast(X) PointerGetDatum(&(X))
-#endif
-
-
static bool
gbt_timegt(const void *a, const void *b, FmgrInfo *flinfo)
{
@@ -45,8 +38,8 @@ gbt_timegt(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_gt,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -56,8 +49,8 @@ gbt_timege(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_ge,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -67,8 +60,8 @@ gbt_timeeq(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_eq,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -78,8 +71,8 @@ gbt_timele(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_le,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static bool
@@ -89,8 +82,8 @@ gbt_timelt(const void *a, const void *b, FmgrInfo *flinfo)
const TimeADT *bb = (const TimeADT *) b;
return DatumGetBool(DirectFunctionCall2(time_lt,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
}
static int
@@ -100,9 +93,9 @@ gbt_timekey_cmp(const void *a, const void *b, FmgrInfo *flinfo)
timeKEY *ib = (timeKEY *) (((const Nsrt *) b)->t);
int res;
- res = DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatumFast(ia->lower),
TimeADTGetDatumFast(ib->lower)));
+ res = DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatum(ia->lower), TimeADTGetDatum(ib->lower)));
if (res == 0)
- return DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatumFast(ia->upper),
TimeADTGetDatumFast(ib->upper)));
+ return DatumGetInt32(DirectFunctionCall2(time_cmp, TimeADTGetDatum(ia->upper), TimeADTGetDatum(ib->upper)));
return res;
}
@@ -115,8 +108,8 @@ gbt_time_dist(const void *a, const void *b, FmgrInfo *flinfo)
Interval *i;
i = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(*aa),
- TimeADTGetDatumFast(*bb)));
+ TimeADTGetDatum(*aa),
+ TimeADTGetDatum(*bb)));
return fabs(INTERVAL_TO_SEC(i));
}
@@ -279,14 +272,14 @@ gbt_time_penalty(PG_FUNCTION_ARGS)
double res2;
intr = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(newentry->upper),
- TimeADTGetDatumFast(origentry->upper)));
+ TimeADTGetDatum(newentry->upper),
+ TimeADTGetDatum(origentry->upper)));
res = INTERVAL_TO_SEC(intr);
res = Max(res, 0);
intr = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(origentry->lower),
- TimeADTGetDatumFast(newentry->lower)));
+ TimeADTGetDatum(origentry->lower),
+ TimeADTGetDatum(newentry->lower)));
res2 = INTERVAL_TO_SEC(intr);
res2 = Max(res2, 0);
@@ -297,8 +290,8 @@ gbt_time_penalty(PG_FUNCTION_ARGS)
if (res > 0)
{
intr = DatumGetIntervalP(DirectFunctionCall2(time_mi_time,
- TimeADTGetDatumFast(origentry->upper),
- TimeADTGetDatumFast(origentry->lower)));
+ TimeADTGetDatum(origentry->upper),
+ TimeADTGetDatum(origentry->lower)));
*result += FLT_MIN;
*result += (float) (res / (res + INTERVAL_TO_SEC(intr)));
*result *= (FLT_MAX / (((GISTENTRY *) PG_GETARG_POINTER(0))->rel->rd_att->natts + 1));
@@ -334,8 +327,8 @@ gbt_timekey_ssup_cmp(Datum x, Datum y, SortSupport ssup)
/* for leaf items we expect lower == upper, so only compare lower */
return DatumGetInt32(DirectFunctionCall2(time_cmp,
- TimeADTGetDatumFast(arg1->lower),
- TimeADTGetDatumFast(arg2->lower)));
+ TimeADTGetDatum(arg1->lower),
+ TimeADTGetDatum(arg2->lower)));
}
Datum
diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index eb899c4d213..1e8f83f0f0a 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -33,13 +33,6 @@ PG_FUNCTION_INFO_V1(gbt_ts_same);
PG_FUNCTION_INFO_V1(gbt_ts_sortsupport);
-#ifdef USE_FLOAT8_BYVAL
-#define TimestampGetDatumFast(X) TimestampGetDatum(X)
-#else
-#define TimestampGetDatumFast(X) PointerGetDatum(&(X))
-#endif
-
-
/* define for comparison */
static bool
@@ -49,8 +42,8 @@ gbt_tsgt(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_gt,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -60,8 +53,8 @@ gbt_tsge(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_ge,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -71,8 +64,8 @@ gbt_tseq(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_eq,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -82,8 +75,8 @@ gbt_tsle(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_le,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static bool
@@ -93,8 +86,8 @@ gbt_tslt(const void *a, const void *b, FmgrInfo *flinfo)
const Timestamp *bb = (const Timestamp *) b;
return DatumGetBool(DirectFunctionCall2(timestamp_lt,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
}
static int
@@ -104,9 +97,9 @@ gbt_tskey_cmp(const void *a, const void *b, FmgrInfo *flinfo)
tsKEY *ib = (tsKEY *) (((const Nsrt *) b)->t);
int res;
- res = DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatumFast(ia->lower),
TimestampGetDatumFast(ib->lower)));
+ res = DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatum(ia->lower),
TimestampGetDatum(ib->lower)));
if (res == 0)
- return DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatumFast(ia->upper),
TimestampGetDatumFast(ib->upper)));
+ return DatumGetInt32(DirectFunctionCall2(timestamp_cmp, TimestampGetDatum(ia->upper),
TimestampGetDatum(ib->upper)));
return res;
}
@@ -122,8 +115,8 @@ gbt_ts_dist(const void *a, const void *b, FmgrInfo *flinfo)
return get_float8_infinity();
i = DatumGetIntervalP(DirectFunctionCall2(timestamp_mi,
- TimestampGetDatumFast(*aa),
- TimestampGetDatumFast(*bb)));
+ TimestampGetDatum(*aa),
+ TimestampGetDatum(*bb)));
return fabs(INTERVAL_TO_SEC(i));
}
@@ -404,8 +397,8 @@ gbt_ts_ssup_cmp(Datum x, Datum y, SortSupport ssup)
/* for leaf items we expect lower == upper, so only compare lower */
return DatumGetInt32(DirectFunctionCall2(timestamp_cmp,
- TimestampGetDatumFast(arg1->lower),
- TimestampGetDatumFast(arg2->lower)));
+ TimestampGetDatum(arg1->lower),
+ TimestampGetDatum(arg2->lower)));
}
Datum
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index be60005ae46..568edacb9bd 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -993,7 +993,7 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
case INT8OID:
att->attlen = 8;
- att->attbyval = FLOAT8PASSBYVAL;
+ att->attbyval = true;
att->attalign = TYPALIGN_DOUBLE;
att->attstorage = TYPSTORAGE_PLAIN;
att->attcompression = InvalidCompressionMethod;
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 219df1971da..1a4f36fe0a9 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -986,11 +986,6 @@ index_store_float8_orderby_distances(IndexScanDesc scan, Oid *orderByTypes,
{
if (orderByTypes[i] == FLOAT8OID)
{
-#ifndef USE_FLOAT8_BYVAL
- /* must free any old value to avoid memory leakage */
- if (!scan->xs_orderbynulls[i])
- pfree(DatumGetPointer(scan->xs_orderbyvals[i]));
-#endif
if (distances && !distances[i].isnull)
{
scan->xs_orderbyvals[i] = Float8GetDatum(distances[i].value);
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 9a4de1616bc..e8909406686 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4390,7 +4390,7 @@ WriteControlFile(void)
ControlFile->toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE;
ControlFile->loblksize = LOBLKSIZE;
- ControlFile->float8ByVal = FLOAT8PASSBYVAL;
+ ControlFile->float8ByVal = true; /* vestigial */
/*
* Initialize the default 'char' signedness.
@@ -4651,23 +4651,7 @@ ReadControlFile(void)
"LOBLKSIZE", (int) LOBLKSIZE),
errhint("It looks like you need to recompile or initdb.")));
-#ifdef USE_FLOAT8_BYVAL
- if (ControlFile->float8ByVal != true)
- ereport(FATAL,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("database files are incompatible with server"),
- errdetail("The database cluster was initialized without USE_FLOAT8_BYVAL"
- " but the server was compiled with USE_FLOAT8_BYVAL."),
- errhint("It looks like you need to recompile or initdb.")));
-#else
- if (ControlFile->float8ByVal != false)
- ereport(FATAL,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("database files are incompatible with server"),
- errdetail("The database cluster was initialized with USE_FLOAT8_BYVAL"
- " but the server was compiled without USE_FLOAT8_BYVAL."),
- errhint("It looks like you need to recompile or initdb.")));
-#endif
+ Assert(ControlFile->float8ByVal); /* vestigial, not worth an error msg */
wal_segment_size = ControlFile->xlog_seg_size;
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index df3231fcd41..6c02aee7267 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -1054,8 +1054,7 @@ sub morph_row_for_schemapg
}
# Expand booleans from 'f'/'t' to 'false'/'true'.
- # Some values might be other macros (eg FLOAT8PASSBYVAL),
- # don't change.
+ # Some values might be other macros, if so don't change.
elsif ($atttype eq 'bool')
{
$row->{$attname} = 'true' if $row->{$attname} eq 't';
diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 64605be3178..2ef0bb7f663 100644
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -410,7 +410,7 @@ build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo,
parse->limitCount = (Node *) makeConst(INT8OID, -1, InvalidOid,
sizeof(int64),
Int64GetDatum(1), false,
- FLOAT8PASSBYVAL);
+ true);
/*
* Generate the best paths for this query, telling query_planner that we
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 5ba0d22befd..0d5a692e5fd 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4915,7 +4915,7 @@ create_partial_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel,
limitCount = (Node *) makeConst(INT8OID, -1, InvalidOid,
sizeof(int64),
Int64GetDatum(1), false,
- FLOAT8PASSBYVAL);
+ true);
/*
* Apply a LimitPath onto the partial path to restrict the
@@ -5118,7 +5118,7 @@ create_final_distinct_paths(PlannerInfo *root, RelOptInfo *input_rel,
limitCount = (Node *) makeConst(INT8OID, -1, InvalidOid,
sizeof(int64),
Int64GetDatum(1), false,
- FLOAT8PASSBYVAL);
+ true);
/*
* If the query already has a LIMIT clause, then we could
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index d6feb16aef3..203b7a32178 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -408,7 +408,7 @@ make_const(ParseState *pstate, A_Const *aconst)
typeid = INT8OID;
typelen = sizeof(int64);
- typebyval = FLOAT8PASSBYVAL; /* int8 and float8 alike */
+ typebyval = true;
}
}
else
diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.c
index 19b89dee0d0..9f95d4dc1b0 100644
--- a/src/backend/rewrite/rewriteSearchCycle.c
+++ b/src/backend/rewrite/rewriteSearchCycle.c
@@ -320,7 +320,7 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
if (cte->search_clause->search_breadth_first)
{
search_col_rowexpr->args = lcons(makeConst(INT8OID, -1, InvalidOid, sizeof(int64),
- Int64GetDatum(0), false, FLOAT8PASSBYVAL),
+ Int64GetDatum(0), false, true),
search_col_rowexpr->args);
search_col_rowexpr->colnames = lcons(makeString("*DEPTH*"), search_col_rowexpr->colnames);
texpr = (Expr *) search_col_rowexpr;
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index c8f53c6fbe7..c833e7df1fd 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -3406,7 +3406,7 @@ construct_array_builtin(Datum *elems, int nelems, Oid elmtype)
case FLOAT8OID:
elmlen = sizeof(float8);
- elmbyval = FLOAT8PASSBYVAL;
+ elmbyval = true;
elmalign = TYPALIGN_DOUBLE;
break;
@@ -3424,7 +3424,7 @@ construct_array_builtin(Datum *elems, int nelems, Oid elmtype)
case INT8OID:
elmlen = sizeof(int64);
- elmbyval = FLOAT8PASSBYVAL;
+ elmbyval = true;
elmalign = TYPALIGN_DOUBLE;
break;
@@ -3718,7 +3718,7 @@ deconstruct_array_builtin(ArrayType *array,
case FLOAT8OID:
elmlen = sizeof(float8);
- elmbyval = FLOAT8PASSBYVAL;
+ elmbyval = true;
elmalign = TYPALIGN_DOUBLE;
break;
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 9dd5889f34c..bdea490202a 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -718,76 +718,29 @@ int8lcm(PG_FUNCTION_ARGS)
Datum
int8inc(PG_FUNCTION_ARGS)
{
- /*
- * When int8 is pass-by-reference, we provide this special case to avoid
- * palloc overhead for COUNT(): when called as an aggregate, we know that
- * the argument is modifiable local storage, so just update it in-place.
- * (If int8 is pass-by-value, then of course this is useless as well as
- * incorrect, so just ifdef it out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *arg = (int64 *) PG_GETARG_POINTER(0);
-
- if (unlikely(pg_add_s64_overflow(*arg, 1, arg)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
-
- PG_RETURN_POINTER(arg);
- }
- else
-#endif
- {
- /* Not called as an aggregate, so just do it the dumb way */
- int64 arg = PG_GETARG_INT64(0);
- int64 result;
+ int64 arg = PG_GETARG_INT64(0);
+ int64 result;
- if (unlikely(pg_add_s64_overflow(arg, 1, &result)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
+ if (unlikely(pg_add_s64_overflow(arg, 1, &result)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
- PG_RETURN_INT64(result);
- }
+ PG_RETURN_INT64(result);
}
Datum
int8dec(PG_FUNCTION_ARGS)
{
- /*
- * When int8 is pass-by-reference, we provide this special case to avoid
- * palloc overhead for COUNT(): when called as an aggregate, we know that
- * the argument is modifiable local storage, so just update it in-place.
- * (If int8 is pass-by-value, then of course this is useless as well as
- * incorrect, so just ifdef it out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *arg = (int64 *) PG_GETARG_POINTER(0);
-
- if (unlikely(pg_sub_s64_overflow(*arg, 1, arg)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
- PG_RETURN_POINTER(arg);
- }
- else
-#endif
- {
- /* Not called as an aggregate, so just do it the dumb way */
- int64 arg = PG_GETARG_INT64(0);
- int64 result;
+ int64 arg = PG_GETARG_INT64(0);
+ int64 result;
- if (unlikely(pg_sub_s64_overflow(arg, 1, &result)))
- ereport(ERROR,
- (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
- errmsg("bigint out of range")));
+ if (unlikely(pg_sub_s64_overflow(arg, 1, &result)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
- PG_RETURN_INT64(result);
- }
+ PG_RETURN_INT64(result);
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 4b94ee9a3b5..f2e1ac50efa 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6363,6 +6363,7 @@ numeric_poly_stddev_pop(PG_FUNCTION_ARGS)
Datum
int2_sum(PG_FUNCTION_ARGS)
{
+ int64 oldsum;
int64 newval;
if (PG_ARGISNULL(0))
@@ -6375,43 +6376,22 @@ int2_sum(PG_FUNCTION_ARGS)
PG_RETURN_INT64(newval);
}
- /*
- * If we're invoked as an aggregate, we can cheat and modify our first
- * parameter in-place to avoid palloc overhead. If not, we need to return
- * the new value of the transition variable. (If int8 is pass-by-value,
- * then of course this is useless as well as incorrect, so just ifdef it
- * out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *oldsum = (int64 *) PG_GETARG_POINTER(0);
+ oldsum = PG_GETARG_INT64(0);
- /* Leave the running sum unchanged in the new input is null */
- if (!PG_ARGISNULL(1))
- *oldsum = *oldsum + (int64) PG_GETARG_INT16(1);
-
- PG_RETURN_POINTER(oldsum);
- }
- else
-#endif
- {
- int64 oldsum = PG_GETARG_INT64(0);
-
- /* Leave sum unchanged if new input is null. */
- if (PG_ARGISNULL(1))
- PG_RETURN_INT64(oldsum);
+ /* Leave sum unchanged if new input is null. */
+ if (PG_ARGISNULL(1))
+ PG_RETURN_INT64(oldsum);
- /* OK to do the addition. */
- newval = oldsum + (int64) PG_GETARG_INT16(1);
+ /* OK to do the addition. */
+ newval = oldsum + (int64) PG_GETARG_INT16(1);
- PG_RETURN_INT64(newval);
- }
+ PG_RETURN_INT64(newval);
}
Datum
int4_sum(PG_FUNCTION_ARGS)
{
+ int64 oldsum;
int64 newval;
if (PG_ARGISNULL(0))
@@ -6424,38 +6404,16 @@ int4_sum(PG_FUNCTION_ARGS)
PG_RETURN_INT64(newval);
}
- /*
- * If we're invoked as an aggregate, we can cheat and modify our first
- * parameter in-place to avoid palloc overhead. If not, we need to return
- * the new value of the transition variable. (If int8 is pass-by-value,
- * then of course this is useless as well as incorrect, so just ifdef it
- * out.)
- */
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
- if (AggCheckCallContext(fcinfo, NULL))
- {
- int64 *oldsum = (int64 *) PG_GETARG_POINTER(0);
+ oldsum = PG_GETARG_INT64(0);
- /* Leave the running sum unchanged in the new input is null */
- if (!PG_ARGISNULL(1))
- *oldsum = *oldsum + (int64) PG_GETARG_INT32(1);
-
- PG_RETURN_POINTER(oldsum);
- }
- else
-#endif
- {
- int64 oldsum = PG_GETARG_INT64(0);
-
- /* Leave sum unchanged if new input is null. */
- if (PG_ARGISNULL(1))
- PG_RETURN_INT64(oldsum);
+ /* Leave sum unchanged if new input is null. */
+ if (PG_ARGISNULL(1))
+ PG_RETURN_INT64(oldsum);
- /* OK to do the addition. */
- newval = oldsum + (int64) PG_GETARG_INT32(1);
+ /* OK to do the addition. */
+ newval = oldsum + (int64) PG_GETARG_INT32(1);
- PG_RETURN_INT64(newval);
- }
+ PG_RETURN_INT64(newval);
}
/*
diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c
index 9457d239715..c41b191be62 100644
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -1007,7 +1007,7 @@ percentile_cont_float8_multi_final(PG_FUNCTION_ARGS)
FLOAT8OID,
/* hard-wired info on type float8 */
sizeof(float8),
- FLOAT8PASSBYVAL,
+ true,
TYPALIGN_DOUBLE,
float8_lerp);
}
diff --git a/src/backend/utils/adt/rangetypes_typanalyze.c b/src/backend/utils/adt/rangetypes_typanalyze.c
index a18196d8a34..36e885af2dd 100644
--- a/src/backend/utils/adt/rangetypes_typanalyze.c
+++ b/src/backend/utils/adt/rangetypes_typanalyze.c
@@ -397,7 +397,7 @@ compute_range_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc,
stats->numvalues[slot_idx] = num_hist;
stats->statypid[slot_idx] = FLOAT8OID;
stats->statyplen[slot_idx] = sizeof(float8);
- stats->statypbyval[slot_idx] = FLOAT8PASSBYVAL;
+ stats->statypbyval[slot_idx] = true;
stats->statypalign[slot_idx] = 'd';
/* Store the fraction of empty ranges */
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 782291d9998..5543440a33e 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -1788,41 +1788,6 @@ OidSendFunctionCall(Oid functionId, Datum val)
}
-/*-------------------------------------------------------------------------
- * Support routines for standard maybe-pass-by-reference datatypes
- *
- * int8 and float8 can be passed by value if Datum is wide enough.
- * (For backwards-compatibility reasons, we allow pass-by-ref to be chosen
- * at compile time even if pass-by-val is possible.)
- *
- * Note: there is only one switch controlling the pass-by-value option for
- * both int8 and float8; this is to avoid making things unduly complicated
- * for the timestamp types, which might have either representation.
- *-------------------------------------------------------------------------
- */
-
-#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
-
-Datum
-Int64GetDatum(int64 X)
-{
- int64 *retval = (int64 *) palloc(sizeof(int64));
-
- *retval = X;
- return PointerGetDatum(retval);
-}
-
-Datum
-Float8GetDatum(float8 X)
-{
- float8 *retval = (float8 *) palloc(sizeof(float8));
-
- *retval = X;
- return PointerGetDatum(retval);
-}
-#endif /* USE_FLOAT8_BYVAL */
-
-
/*-------------------------------------------------------------------------
* Support routines for toastable datatypes
*-------------------------------------------------------------------------
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 62bbd08d9f6..92fe2f531f7 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1580,9 +1580,6 @@ bootstrap_template1(void)
bki_lines = replace_token(bki_lines, "ALIGNOF_POINTER",
(sizeof(Pointer) == 4) ? "i" : "d");
- bki_lines = replace_token(bki_lines, "FLOAT8PASSBYVAL",
- FLOAT8PASSBYVAL ? "true" : "false");
-
bki_lines = replace_token(bki_lines, "POSTGRES",
escape_quotes_bki(username));
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index e876f35f38e..7a4e4eb9570 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -719,7 +719,7 @@ GuessControlValues(void)
ControlFile.indexMaxKeys = INDEX_MAX_KEYS;
ControlFile.toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE;
ControlFile.loblksize = LOBLKSIZE;
- ControlFile.float8ByVal = FLOAT8PASSBYVAL;
+ ControlFile.float8ByVal = true; /* vestigial */
/*
* XXX eventually, should try to grovel through old XLOG to develop more
diff --git a/src/include/c.h b/src/include/c.h
index bbdaa88c63a..39022f8a9dd 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -609,11 +609,11 @@ typedef signed int Offset;
typedef float float4;
typedef double float8;
-#ifdef USE_FLOAT8_BYVAL
+/*
+ * float8, int8, and related datatypes are now always pass-by-value.
+ * We keep this symbol to avoid breaking extension code that may use it.
+ */
#define FLOAT8PASSBYVAL true
-#else
-#define FLOAT8PASSBYVAL false
-#endif
/*
* Oid, RegProcedure, TransactionId, SubTransactionId, MultiXactId,
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index 29e4ffffc98..cb730aeac86 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -54,7 +54,7 @@
typcollation => 'C' },
{ oid => '20', array_type_oid => '1016',
descr => '~18 digit integer, 8-byte storage',
- typname => 'int8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'int8', typlen => '8', typbyval => 't',
typcategory => 'N', typinput => 'int8in', typoutput => 'int8out',
typreceive => 'int8recv', typsend => 'int8send', typalign => 'd' },
{ oid => '21', array_type_oid => '1005',
@@ -172,7 +172,7 @@
typoutput => 'pg_ddl_command_out', typreceive => 'pg_ddl_command_recv',
typsend => 'pg_ddl_command_send', typalign => 'ALIGNOF_POINTER' },
{ oid => '5069', array_type_oid => '271', descr => 'full transaction id',
- typname => 'xid8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'xid8', typlen => '8', typbyval => 't',
typcategory => 'U', typinput => 'xid8in', typoutput => 'xid8out',
typreceive => 'xid8recv', typsend => 'xid8send', typalign => 'd' },
@@ -222,7 +222,7 @@
typsend => 'float4send', typalign => 'i' },
{ oid => '701', array_type_oid => '1022',
descr => 'double-precision floating point number, 8-byte storage',
- typname => 'float8', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'float8', typlen => '8', typbyval => 't',
typcategory => 'N', typispreferred => 't', typinput => 'float8in',
typoutput => 'float8out', typreceive => 'float8recv', typsend => 'float8send',
typalign => 'd' },
@@ -237,7 +237,7 @@
typreceive => 'circle_recv', typsend => 'circle_send', typalign => 'd' },
{ oid => '790', array_type_oid => '791',
descr => 'monetary amounts, $d,ddd.cc',
- typname => 'money', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'money', typlen => '8', typbyval => 't',
typcategory => 'N', typinput => 'cash_in', typoutput => 'cash_out',
typreceive => 'cash_recv', typsend => 'cash_send', typalign => 'd' },
@@ -290,7 +290,7 @@
typinput => 'date_in', typoutput => 'date_out', typreceive => 'date_recv',
typsend => 'date_send', typalign => 'i' },
{ oid => '1083', array_type_oid => '1183', descr => 'time of day',
- typname => 'time', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'time', typlen => '8', typbyval => 't',
typcategory => 'D', typinput => 'time_in', typoutput => 'time_out',
typreceive => 'time_recv', typsend => 'time_send', typmodin => 'timetypmodin',
typmodout => 'timetypmodout', typalign => 'd' },
@@ -298,14 +298,14 @@
# OIDS 1100 - 1199
{ oid => '1114', array_type_oid => '1115', descr => 'date and time',
- typname => 'timestamp', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'timestamp', typlen => '8', typbyval => 't',
typcategory => 'D', typinput => 'timestamp_in', typoutput => 'timestamp_out',
typreceive => 'timestamp_recv', typsend => 'timestamp_send',
typmodin => 'timestamptypmodin', typmodout => 'timestamptypmodout',
typalign => 'd' },
{ oid => '1184', array_type_oid => '1185',
descr => 'date and time with time zone',
- typname => 'timestamptz', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'timestamptz', typlen => '8', typbyval => 't',
typcategory => 'D', typispreferred => 't', typinput => 'timestamptz_in',
typoutput => 'timestamptz_out', typreceive => 'timestamptz_recv',
typsend => 'timestamptz_send', typmodin => 'timestamptztypmodin',
@@ -413,7 +413,7 @@
# pg_lsn
{ oid => '3220', array_type_oid => '3221', descr => 'PostgreSQL LSN',
- typname => 'pg_lsn', typlen => '8', typbyval => 'FLOAT8PASSBYVAL',
+ typname => 'pg_lsn', typlen => '8', typbyval => 't',
typcategory => 'U', typinput => 'pg_lsn_in', typoutput => 'pg_lsn_out',
typreceive => 'pg_lsn_recv', typsend => 'pg_lsn_send', typalign => 'd' },
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 0fe7b4ebc77..c7236e42972 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -469,7 +469,7 @@ typedef struct
int funcmaxargs; /* FUNC_MAX_ARGS */
int indexmaxkeys; /* INDEX_MAX_KEYS */
int namedatalen; /* NAMEDATALEN */
- int float8byval; /* FLOAT8PASSBYVAL */
+ int float8byval; /* FLOAT8PASSBYVAL (now vestigial) */
char abi_extra[32]; /* see pg_config_manual.h */
} Pg_abi_values;
diff --git a/src/include/postgres.h b/src/include/postgres.h
index e81829bfa6f..357cbd6fd96 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -388,68 +388,41 @@ NameGetDatum(const NameData *X)
/*
* DatumGetInt64
* Returns 64-bit integer value of a datum.
- *
- * Note: this function hides whether int64 is pass by value or by reference.
*/
static inline int64
DatumGetInt64(Datum X)
{
-#ifdef USE_FLOAT8_BYVAL
return (int64) X;
-#else
- return *((int64 *) DatumGetPointer(X));
-#endif
}
/*
* Int64GetDatum
* Returns datum representation for a 64-bit integer.
- *
- * Note: if int64 is pass by reference, this function returns a reference
- * to palloc'd space.
*/
-#ifdef USE_FLOAT8_BYVAL
static inline Datum
Int64GetDatum(int64 X)
{
return (Datum) X;
}
-#else
-extern Datum Int64GetDatum(int64 X);
-#endif
-
/*
* DatumGetUInt64
* Returns 64-bit unsigned integer value of a datum.
- *
- * Note: this function hides whether int64 is pass by value or by reference.
*/
static inline uint64
DatumGetUInt64(Datum X)
{
-#ifdef USE_FLOAT8_BYVAL
return (uint64) X;
-#else
- return *((uint64 *) DatumGetPointer(X));
-#endif
}
/*
* UInt64GetDatum
* Returns datum representation for a 64-bit unsigned integer.
- *
- * Note: if int64 is pass by reference, this function returns a reference
- * to palloc'd space.
*/
static inline Datum
UInt64GetDatum(uint64 X)
{
-#ifdef USE_FLOAT8_BYVAL
return (Datum) X;
-#else
- return Int64GetDatum((int64) X);
-#endif
}
/*
@@ -497,13 +470,10 @@ Float4GetDatum(float4 X)
/*
* DatumGetFloat8
* Returns 8-byte floating point value of a datum.
- *
- * Note: this function hides whether float8 is pass by value or by reference.
*/
static inline float8
DatumGetFloat8(Datum X)
{
-#ifdef USE_FLOAT8_BYVAL
union
{
int64 value;
@@ -512,19 +482,12 @@ DatumGetFloat8(Datum X)
myunion.value = DatumGetInt64(X);
return myunion.retval;
-#else
- return *((float8 *) DatumGetPointer(X));
-#endif
}
/*
* Float8GetDatum
* Returns datum representation for an 8-byte floating point number.
- *
- * Note: if float8 is pass by reference, this function returns a reference
- * to palloc'd space.
*/
-#ifdef USE_FLOAT8_BYVAL
static inline Datum
Float8GetDatum(float8 X)
{
@@ -537,35 +500,22 @@ Float8GetDatum(float8 X)
myunion.value = X;
return Int64GetDatum(myunion.retval);
}
-#else
-extern Datum Float8GetDatum(float8 X);
-#endif
-
/*
* Int64GetDatumFast
* Float8GetDatumFast
*
- * These macros are intended to allow writing code that does not depend on
+ * These macros were intended to allow writing code that does not depend on
* whether int64 and float8 are pass-by-reference types, while not
- * sacrificing performance when they are. The argument must be a variable
- * that will exist and have the same value for as long as the Datum is needed.
- * In the pass-by-ref case, the address of the variable is taken to use as
- * the Datum. In the pass-by-val case, these are the same as the non-Fast
- * functions, except for asserting that the variable is of the correct type.
+ * sacrificing performance when they are. They are no longer different
+ * from the regular functions, though we keep the assertions to protect
+ * code that might get back-patched into older branches.
*/
-#ifdef USE_FLOAT8_BYVAL
#define Int64GetDatumFast(X) \
(AssertVariableIsOfTypeMacro(X, int64), Int64GetDatum(X))
#define Float8GetDatumFast(X) \
(AssertVariableIsOfTypeMacro(X, double), Float8GetDatum(X))
-#else
-#define Int64GetDatumFast(X) \
- (AssertVariableIsOfTypeMacro(X, int64), PointerGetDatum(&(X)))
-#define Float8GetDatumFast(X) \
- (AssertVariableIsOfTypeMacro(X, double), PointerGetDatum(&(X)))
-#endif
/* ----------------------------------------------------------------
--
2.43.7
On 12.08.25 21:48, Tom Lane wrote: > PFA v2 with these changes. I feel like this is pretty close to > committable now. This looks good to me. One additional comment: In network.c and varlena.c there are a couple of comments that match /4 byte datum/ that talk about abbreviation support in that circumstance. Those should probably be removed or reworded.
Peter Eisentraut <peter@eisentraut.org> writes:
> One additional comment: In network.c and varlena.c there are a couple of
> comments that match /4 byte datum/ that talk about abbreviation support
> in that circumstance. Those should probably be removed or reworded.
Ah, didn't think to search for that. I'll take care of it.
Thanks again for reviewing.
regards, tom lane
Hi,
While testing a different patch, I tried running with address sanitizer
on rpi5, running the 32-bit OS (which AFAIK is 64-bit kernel and 32-bit
user space). With that, stats_ext regression tests fail like this:
extended_stats.c:1082:27: runtime error: store to misaligned address
0x036671dc for type 'Datum', which requires 8 byte alignment
0x036671dc: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 7e
7f 08 00 00 00 7f 7f 7f 7f
^
This happens because build_sorted_items() does palloc(), and then
accesses the pointer as array of structs, with a Datum field. And it
apparently expects the pointer to be a multiple of 8 bytes. Isn't that a
bit strange, with 32-bit user space? The pointer is indeed a multiple of
4B, so maybe the expected alignment is wrong?
I did try this on REL_18_STABLE, and that works just fine, so I believe
it's about this commit. I also tried this on a i386 debian environment
(more precisely, it's 32-bit chroot on 64-bit system, created using
debootstrap). And that seems to work fine too ...
It's entirely possible this is a rpi5-specific issue, or maybe a kernel
issue. The last time we saw something similar weirdness, it turned out
to be a long-standing kernel bug in move_pages(). But that affected the
x86 systems too.
FWIW this is how I run with address sanitizer:
./configure --enable-debug --enable-cassert \
CPPFLAGS="-O0 -fsanitize=alignment -fno-sanitize-recover=all -latomic" \
LDFLAGS="-fsanitize=alignment -latomic"
regards
--
Tomas Vondra
Tomas Vondra <tomas@vondra.me> writes:
> While testing a different patch, I tried running with address sanitizer
> on rpi5, running the 32-bit OS (which AFAIK is 64-bit kernel and 32-bit
> user space). With that, stats_ext regression tests fail like this:
> extended_stats.c:1082:27: runtime error: store to misaligned address
> 0x036671dc for type 'Datum', which requires 8 byte alignment
> 0x036671dc: note: pointer points here
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 7e
> 7f 08 00 00 00 7f 7f 7f 7f
> ^
> This happens because build_sorted_items() does palloc(), and then
> accesses the pointer as array of structs, with a Datum field. And it
> apparently expects the pointer to be a multiple of 8 bytes. Isn't that a
> bit strange, with 32-bit user space? The pointer is indeed a multiple of
> 4B, so maybe the expected alignment is wrong?
I think build_sorted_items is plainly at fault here, where it does
/* Compute the total amount of memory we need (both items and values). */
len = data->numrows * sizeof(SortItem) + nvalues * (sizeof(Datum) + sizeof(bool));
/* Allocate the memory and split it into the pieces. */
ptr = palloc0(len);
/* items to sort */
items = (SortItem *) ptr;
ptr += data->numrows * sizeof(SortItem);
/* values and null flags */
values = (Datum *) ptr;
ptr += nvalues * sizeof(Datum);
This is silently assuming that sizeof(SortItem) is a multiple of
alignof(Datum), which on a 32-bit-pointer platform is not true
any longer. We ought to MAXALIGN the two occurrences of
data->numrows * sizeof(SortItem).
Do you want to fix it, or shall I?
regards, tom lane
On 9/10/25 22:35, Tom Lane wrote: > Tomas Vondra <tomas@vondra.me> writes: >> While testing a different patch, I tried running with address sanitizer >> on rpi5, running the 32-bit OS (which AFAIK is 64-bit kernel and 32-bit >> user space). With that, stats_ext regression tests fail like this: > >> extended_stats.c:1082:27: runtime error: store to misaligned address >> 0x036671dc for type 'Datum', which requires 8 byte alignment >> 0x036671dc: note: pointer points here >> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 7e >> 7f 08 00 00 00 7f 7f 7f 7f >> ^ > >> This happens because build_sorted_items() does palloc(), and then >> accesses the pointer as array of structs, with a Datum field. And it >> apparently expects the pointer to be a multiple of 8 bytes. Isn't that a >> bit strange, with 32-bit user space? The pointer is indeed a multiple of >> 4B, so maybe the expected alignment is wrong? > > I think build_sorted_items is plainly at fault here, where it does > > /* Compute the total amount of memory we need (both items and values). */ > len = data->numrows * sizeof(SortItem) + nvalues * (sizeof(Datum) + sizeof(bool)); > > /* Allocate the memory and split it into the pieces. */ > ptr = palloc0(len); > > /* items to sort */ > items = (SortItem *) ptr; > ptr += data->numrows * sizeof(SortItem); > > /* values and null flags */ > values = (Datum *) ptr; > ptr += nvalues * sizeof(Datum); > > This is silently assuming that sizeof(SortItem) is a multiple of > alignof(Datum), which on a 32-bit-pointer platform is not true > any longer. We ought to MAXALIGN the two occurrences of > data->numrows * sizeof(SortItem). > You're right, I misunderstood which of the accesses is triggering the report. I added the two MAXALIGNs and can confirm that makes it go away on the rpi5. It's interesting it didn't happen on the i386 machine at all, but I don't have time to look at why right now. > Do you want to fix it, or shall I? > Feel free to do so. If not, I'll do that on Monday. Thanks -- Tomas Vondra
Tomas Vondra <tomas@vondra.me> writes:
> On 9/10/25 22:35, Tom Lane wrote:
>> This is silently assuming that sizeof(SortItem) is a multiple of
>> alignof(Datum), which on a 32-bit-pointer platform is not true
>> any longer. We ought to MAXALIGN the two occurrences of
>> data->numrows * sizeof(SortItem).
> You're right, I misunderstood which of the accesses is triggering the
> report. I added the two MAXALIGNs and can confirm that makes it go away
> on the rpi5. It's interesting it didn't happen on the i386 machine at
> all, but I don't have time to look at why right now.
I think it's just that i386 doesn't have hardware-level alignment
restrictions, or at least not ones for more than 4 bytes.
>> Do you want to fix it, or shall I?
> Feel free to do so. If not, I'll do that on Monday.
I can deal with it today, will go do so.
regards, tom lane
Em qua., 10 de set. de 2025 às 17:35, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Tomas Vondra <tomas@vondra.me> writes:
> While testing a different patch, I tried running with address sanitizer
> on rpi5, running the 32-bit OS (which AFAIK is 64-bit kernel and 32-bit
> user space). With that, stats_ext regression tests fail like this:
> extended_stats.c:1082:27: runtime error: store to misaligned address
> 0x036671dc for type 'Datum', which requires 8 byte alignment
> 0x036671dc: note: pointer points here
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 7e
> 7f 08 00 00 00 7f 7f 7f 7f
> ^
> This happens because build_sorted_items() does palloc(), and then
> accesses the pointer as array of structs, with a Datum field. And it
> apparently expects the pointer to be a multiple of 8 bytes. Isn't that a
> bit strange, with 32-bit user space? The pointer is indeed a multiple of
> 4B, so maybe the expected alignment is wrong?
I think build_sorted_items is plainly at fault here, where it does
/* Compute the total amount of memory we need (both items and values). */
len = data->numrows * sizeof(SortItem) + nvalues * (sizeof(Datum) + sizeof(bool));
/* Allocate the memory and split it into the pieces. */
ptr = palloc0(len);
/* items to sort */
items = (SortItem *) ptr;
ptr += data->numrows * sizeof(SortItem);
/* values and null flags */
values = (Datum *) ptr;
ptr += nvalues * sizeof(Datum);
This is silently assuming that sizeof(SortItem) is a multiple of
alignof(Datum), which on a 32-bit-pointer platform is not true
any longer. We ought to MAXALIGN the two occurrences of
data->numrows * sizeof(SortItem).
We possibly have two more instances?
1. Function ndistinct_for_combination (src/backend/statistics/mvdistinct.c)
- items = (SortItem *) palloc(numrows * sizeof(SortItem));
+ items = (SortItem *) palloc(MAXALIGN(numrows * sizeof(SortItem)));
+ items = (SortItem *) palloc(MAXALIGN(numrows * sizeof(SortItem)));
2. Function build_distinct_groups (src/backend/statistics/mcv.c)
- SortItem *groups = (SortItem *) palloc(ngroups * sizeof(SortItem));
+ SortItem *groups = (SortItem *) palloc(MAXALIGN(ngroups * sizeof(SortItem)));
+ SortItem *groups = (SortItem *) palloc(MAXALIGN(ngroups * sizeof(SortItem)));
best regards,
Ranier Vilela
On Thu, Jul 17, 2025 at 8:10 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > In a discussion on Discord (in the PG #core-hacking channel, > which unfortunately is inaccessible to non-members), Andres > and Robert complained about the development/maintenance costs > of continuing to support 32-bit platforms. Here is a modest > proposal to reduce those costs without going so far as to > entirely desupport such platforms: let's require them to use > 8-byte Datums even though that's probably not a native data > type for them. That lets us get rid of logic to support the > !USE_FLOAT8_BYVAL case, and allows a few other simplifications. > > The attached patch switches to 8-byte Datums everywhere, but > doesn't make any effort to remove the now-dead code. I made > it just as a proof-of-concept that this can work. It compiled > cleanly and passed check-world for me on a 32-bit FreeBSD > image. Sorry for not responding to this thread sooner, but thanks, Tom. I think this is a great change and I appreciate you doing the legwork. -- Robert Haas EDB: http://www.enterprisedb.com
Ranier Vilela <ranier.vf@gmail.com> writes:
> Em qua., 10 de set. de 2025 às 17:35, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
>> This is silently assuming that sizeof(SortItem) is a multiple of
>> alignof(Datum), which on a 32-bit-pointer platform is not true
>> any longer. We ought to MAXALIGN the two occurrences of
>> data->numrows * sizeof(SortItem).
> We possibly have two more instances?
> 1. Function ndistinct_for_combination (src/backend/statistics/mvdistinct.c)
> - items = (SortItem *) palloc(numrows * sizeof(SortItem));
> + items = (SortItem *) palloc(MAXALIGN(numrows * sizeof(SortItem)));
> 2. Function build_distinct_groups (src/backend/statistics/mcv.c)
> - SortItem *groups = (SortItem *) palloc(ngroups * sizeof(SortItem));
> + SortItem *groups = (SortItem *) palloc(MAXALIGN(ngroups *
> sizeof(SortItem)));
Neither of those have any hazard, because they are not trying to
allocate multiple arrays using address arithmetic. The part of
build_sorted_items that was actually problematic was doing
ptr += data->numrows * sizeof(SortItem);
and then assuming that the result was suitably aligned to be
cast to Datum*.
regards, tom lane
Em qui., 11 de set. de 2025 às 12:36, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:
> Em qua., 10 de set. de 2025 às 17:35, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
>> This is silently assuming that sizeof(SortItem) is a multiple of
>> alignof(Datum), which on a 32-bit-pointer platform is not true
>> any longer. We ought to MAXALIGN the two occurrences of
>> data->numrows * sizeof(SortItem).
> We possibly have two more instances?
> 1. Function ndistinct_for_combination (src/backend/statistics/mvdistinct.c)
> - items = (SortItem *) palloc(numrows * sizeof(SortItem));
> + items = (SortItem *) palloc(MAXALIGN(numrows * sizeof(SortItem)));
> 2. Function build_distinct_groups (src/backend/statistics/mcv.c)
> - SortItem *groups = (SortItem *) palloc(ngroups * sizeof(SortItem));
> + SortItem *groups = (SortItem *) palloc(MAXALIGN(ngroups *
> sizeof(SortItem)));
Neither of those have any hazard, because they are not trying to
allocate multiple arrays using address arithmetic. The part of
build_sorted_items that was actually problematic was doing
ptr += data->numrows * sizeof(SortItem);
and then assuming that the result was suitably aligned to be
cast to Datum*.
Thanks Tom, for double checking.
best regards,
Ranier Vilela