Re: pgaio_io_get_id() type (was Re: Datum as struct)

On 08.08.25 22:07, Andres Freund wrote: On 2025-08-05 19:20:20 +0200, Peter Eisentraut wrote: On 31.07.25 19:17, Tom Lane wrote: Also I see a "// XXX" in pg_get_aios, which I guess is a note to confirm the data type to use for ioh_id? Yes, the stuff returned from pgaio_io_get_id() should be i

Re: Datum as struct

Peter Eisentraut writes: > On 08.08.25 22:30, Andres Freund wrote: >> One thing that would be an interesting follow-up would be to make the struct >> not just carry the datum, but also the type of the field, to be filled in by >> tuple deforming and the *GetDatum() functions. Then we could assert

Re: Datum as struct

On 08.08.25 22:30, Andres Freund wrote: One thing that would be an interesting follow-up would be to make the struct not just carry the datum, but also the type of the field, to be filled in by tuple deforming and the *GetDatum() functions. Then we could assert that the correct DatumGet*() functi

Re: Datum as struct

On 08.08.25 22:55, Tom Lane wrote: FWIW, I don't love the "DummyDatum" name either. Maybe InvalidDatum? I specifically did not use InvalidDatum because, for example, the result of Int32GetDatum(0) is by no means "invalid". Maybe something like NullDatum or VoidDatum? A few related thought

Re: Datum as struct

Andres Freund writes: > On 2025-07-31 16:02:35 +0200, Peter Eisentraut wrote: >> diff --git a/contrib/ltree/_ltree_gist.c b/contrib/ltree/_ltree_gist.c >> index 286ad24fbe8..2d71cea7e5a 100644 >> --- a/contrib/ltree/_ltree_gist.c >> +++ b/contrib/ltree/_ltree_gist.c >> @@ -84,7 +84,7 @@ _ltree_com

Re: Datum as struct

Hi, On 2025-07-31 16:02:35 +0200, Peter Eisentraut wrote: > Another draft patch set that I had lying around that was mentioned in [0]. Nice, thanks for doing that. I tried this a few years back and was scared away by the churn, but I think it's well worth tackling. One thing that would be an int

Re: pgaio_io_get_id() type (was Re: Datum as struct)

Hi, On 2025-08-05 19:20:20 +0200, Peter Eisentraut wrote: > On 31.07.25 19:17, Tom Lane wrote: > > Also I see a "// XXX" in pg_get_aios, which I guess is a note > > to confirm the data type to use for ioh_id? > > Yes, the stuff returned from pgaio_io_get_id() should be int, but some code > uses u

Re: Datum as struct

On 07.08.25 03:55, Tom Lane wrote: I wrote: I think that on a 32-bit machine this would actually result in a null-pointer core dump, since the 0.0 would be coerced to a zero Datum value. The line is not reached in our regression tests, and given the lack of field complaints, it must be hard to

Re: Datum as struct

I wrote: > I think that on a 32-bit machine this would actually result in a > null-pointer core dump, since the 0.0 would be coerced to a zero > Datum value. The line is not reached in our regression tests, > and given the lack of field complaints, it must be hard to reach > in normal use too. Or

Re: Datum as struct

Peter Eisentraut writes: >> On 05.08.25 20:06, Peter Eisentraut wrote: >>> -    PG_RETURN_TIMESTAMPTZ(timestamp); >>> +    return timestamp; >> This one is an actual bug. I took another look through this patch series and realized that this bit from 0003 is also a

Re: Datum as struct

On 05.08.25 20:06, Peter Eisentraut wrote: > diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/ adt/timestamp.c > index 25cff56c3d0..e640b48205b 100644 > --- a/src/backend/utils/adt/timestamp.c > +++ b/src/backend/utils/adt/timestamp.c > @@ -4954,7 +4954,7 @@ timestamptz_tr

Re: Datum as struct

> diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c > index 25cff56c3d0..e640b48205b 100644 > --- a/src/backend/utils/adt/timestamp.c > +++ b/src/backend/utils/adt/timestamp.c > @@ -4954,7 +4954,7 @@ timestamptz_trunc_internal(text *units, TimestampTz timestamp,

pgaio_io_get_id() type (was Re: Datum as struct)

On 31.07.25 19:17, Tom Lane wrote: Also I see a "// XXX" in pg_get_aios, which I guess is a note to confirm the data type to use for ioh_id? Yes, the stuff returned from pgaio_io_get_id() should be int, but some code uses uint32, and also UINT32_MAX as a special marker. Here is a patch that

Re: Datum as struct

On Thu, Jul 31, 2025 at 01:17:54PM -0400, Tom Lane wrote: > My only reservation about 0004 is whether we want to do it like > this, or with the separate "_D" functions that I proposed in > the other thread. Right now the vote seems to be 2 to 1 for > adding DatumGetPointer calls, so unless there a

Re: Datum as struct

Peter Eisentraut writes: > But I think the patches 0001 through 0005 are useful now. > I tested this against the original patch in [0]. It fixes some of the > issues discussed there but not all of them. I looked through this. I think 0001-0003 are unconditionally improvements and you should ju