pgrx icon indicating copy to clipboard operation
pgrx copied to clipboard

Pass 64-bit types by reference on 32-bit platforms

Open CoderPuppy opened this issue 1 year ago • 1 comments

Int64, UInt64 and Float8 are all passed by reference when the pointer size is less than 64 bit. In Postgres this is controlled by the USE_FLOAT8_BYVAL macro defined in pg_config_manual.h, which is used when defining DatumGetInt64/UIn64/Float8 and Int64/UInt64/Float8GetDatum in postgres.h (and propagated to FLOAT8PASSBYVAL). The same logic should be implemented in IntoDatum and FromDatum for i64, u64 and f64.

CoderPuppy avatar Apr 29 '24 16:04 CoderPuppy

It is documented in the system requirements that we do not support 32-bit systems.

While it mentions some examples, these are not the only cases that assume a 64-bit Datum type. Making pgrx work on 32-bit very well requires more than these changes, and it requires pervasive changes, because we e.g. provide a pg_sys::Datum::from(i64). That cannot work except by truncating data on a 32-bit system.

While we have merged changes that make it a little easier to run pgrx on a 32-bit system before, this is the point where things become more than incidentally adding support. I really want to know why we'd be adding support. Finishing 32-bit support is going to cost hundreds of lines of code of revision and review.

And I'd want real tests, here, that I can run while on a 64-bit system, because otherwise I'm going to break it again. With a test suite that is a command away, I at least can lean on those.

workingjubilee avatar Apr 30 '24 20:04 workingjubilee

Right, so, I'm sorry but we can't go to the considerable effort of supporting this (and it requires much more pervasive changes than just #1686 offered) without tests and considerable investment back. Closing this because this will never transpire unless someone is willing to walk the entire walk... I would expect it to take months, plural, to hammer down all the bugs, because it's not just a small change here and there.

workingjubilee avatar Aug 14 '24 20:08 workingjubilee

co-signed

eeeebbbbrrrr avatar Aug 14 '24 20:08 eeeebbbbrrrr