Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Avoid faulty alignment of Datums in build_sorted_items().

  2. Grab the low-hanging fruit from forcing USE_FLOAT8_BYVAL to true.

  3. Grab the low-hanging fruit from forcing sizeof(Datum) to 8.

  4. Make type Datum be 8 bytes wide everywhere.

  5. Mop-up for Datum conversion cleanups.

  1. Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-18T00:09:57Z

    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

  2. Re: Making type Datum be 8 bytes everywhere

    Andres Freund <andres@anarazel.de> — 2025-07-18T16:26:38Z

    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 different size [-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 different size [-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 integer of 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 different size [-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

  3. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-18T17:24:32Z

    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 different size [-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 different size [-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

  4. Re: Making type Datum be 8 bytes everywhere

    Andres Freund <andres@anarazel.de> — 2025-07-23T18:03:06Z

    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 different size [-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 different size [-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

  5. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-23T19:00:04Z

    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

  6. Re: Making type Datum be 8 bytes everywhere

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2025-07-24T11:58:47Z

    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

  7. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-30T15:48:08Z

    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

  8. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-30T16:06:31Z

    [ 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

  9. Re: Making type Datum be 8 bytes everywhere

    Peter Eisentraut <peter@eisentraut.org> — 2025-07-31T14:13:10Z

    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

  10. Re: Making type Datum be 8 bytes everywhere

    Peter Eisentraut <peter@eisentraut.org> — 2025-07-31T14:17:06Z

    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().

  11. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-31T14:27:37Z

    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

  12. Re: Making type Datum be 8 bytes everywhere

    Peter Eisentraut <peter@eisentraut.org> — 2025-07-31T14:30:44Z

    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!?!)

  13. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-31T15:18:03Z

    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

  14. Re: Making type Datum be 8 bytes everywhere

    Michael Paquier <michael@paquier.xyz> — 2025-07-31T23:28:47Z

    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

  15. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-08-09T01:14:08Z

    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

  16. Re: Making type Datum be 8 bytes everywhere

    Joe Conway <mail@joeconway.com> — 2025-08-09T11:45:16Z

    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

  17. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-08-09T16:44:51Z

    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

  18. Re: Making type Datum be 8 bytes everywhere

    Andrew Dunstan <andrew@dunslane.net> — 2025-08-09T18:54:11Z

    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

  19. Re: Making type Datum be 8 bytes everywhere

    Peter Eisentraut <peter@eisentraut.org> — 2025-08-12T06:30:43Z

    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.

  20. Re: Making type Datum be 8 bytes everywhere

    Andres Freund <andres@anarazel.de> — 2025-08-12T14:11:15Z

    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

  21. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-08-12T16:20:58Z

    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

  22. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-08-12T19:48:50Z

    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

  23. Re: Making type Datum be 8 bytes everywhere

    Peter Eisentraut <peter@eisentraut.org> — 2025-08-13T19:29:59Z

    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.

  24. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-08-13T20:04:06Z

    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

  25. Re: Making type Datum be 8 bytes everywhere

    Tomas Vondra <tomas@vondra.me> — 2025-09-10T20:27:57Z

    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

  26. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-10T20:35:42Z

    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

  27. Re: Making type Datum be 8 bytes everywhere

    Tomas Vondra <tomas@vondra.me> — 2025-09-10T21:31:52Z

    
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

  28. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-10T21:35:04Z

    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

  29. Re: Making type Datum be 8 bytes everywhere

    Ranier Vilela <ranier.vf@gmail.com> — 2025-09-11T12:43:35Z

    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)));

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)));

best regards,
Ranier Vilela

  30. Re: Making type Datum be 8 bytes everywhere

    Robert Haas <robertmhaas@gmail.com> — 2025-09-11T13:14:24Z

    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

  31. Re: Making type Datum be 8 bytes everywhere

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-11T15:36:20Z

    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

  32. Re: Making type Datum be 8 bytes everywhere

    Ranier Vilela <ranier.vf@gmail.com> — 2025-09-11T16:15:31Z

    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