pgrx icon indicating copy to clipboard operation
pgrx copied to clipboard

Using `TimestampTz` produces SQL function signatures with incorrect type

Open LucaCappelletti94 opened this issue 7 months ago • 2 comments

At the time of writing, a Timestamp with Time Zone is handled as a numerical value, which is of course exactly what postgres does.

pub type TimestampTz = int64;

But it also means that the current approach to generate a function which receives as arguments TimestampTz, or more specifically with the same OID, will return pgrx::pg_sys::TimestampTz::type_oid(), which is a bigint (20) and not Timezone with timestamp (1184).

As such, it produces an SQL function with the following signature:

/* <begin connected objects> */
-- web/web_common/pgrx_validation/src/chrono.rs:5
-- pgrx_validation::chrono::must_be_smaller_than_utc
CREATE  FUNCTION "must_be_smaller_than_utc"(
	"left" bigint,
	"right" bigint
) RETURNS bool /* bool */
STRICT
LANGUAGE c /* Rust */
AS 'MODULE_PATHNAME', 'must_be_smaller_than_utc_wrapper';
/* </end connected objects> */

and when used with a column of time TIMESTAMP WITH TIME ZONE, I get the error: "function must_be_smaller_than_utc(timestamp with time zone, timestamp with time zone) does not exist"

I am not certain what may be the preferable solution to this issue in the pgrx library - at this time, I am making a wrapper to handle it in my own use case.

Specifically, for future internauts finding themselves in the same pickle, implement the SqlTranslatable trait for my wrapper as follows:

unsafe impl SqlTranslatable for crate::TimestampUTC {
    fn argument_sql() -> Result<SqlMapping, ArgumentError> {
        Ok(SqlMapping::literal("timestamp with time zone"))
    }
    fn return_sql() -> Result<Returns, ReturnsError> {
        Ok(Returns::One(SqlMapping::literal("timestamp with time zone")))
    }
}

LucaCappelletti94 avatar May 31 '25 13:05 LucaCappelletti94

Dang, that's unfortunate. We'll need to make a newtype wrapper instead for pg_sys::TimestampTz and give it a treatment similar to how we've done pg_sys::Oid and pg_sys::TransactionId and similar.

eeeebbbbrrrr avatar May 31 '25 16:05 eeeebbbbrrrr

Would it be a adequate solution to use, instead of a wrapper type, plainly chrono structs? That's what I am wrapping in my own use case.

LucaCappelletti94 avatar Jun 07 '25 11:06 LucaCappelletti94

Would it be a adequate solution to use, instead of a wrapper type, plainly chrono structs? That's what I am wrapping in my own use case.

No, chrono is not an option. We once used chrono here and it was problematic for a lot of reasons.

That said, why aren't you using pgrx::datum::TimestampWithTimeZone? It does all the things. I think my comment above is ill-advised, actually.

eeeebbbbrrrr avatar Jun 28 '25 14:06 eeeebbbbrrrr

Which problems did you encounter? Is there an issue relative to it?

I wasn't using pgrx::datum::TimestampWithTimeZone because I wasn't aware there was another struct other than pgrx::pg_sys::TimestampTZ honestly. It may fit the use case.

LucaCappelletti94 avatar Jun 28 '25 14:06 LucaCappelletti94

@LucaCappelletti94 It could create some extreme conversion overhead when dealing with large quantities of the type.

workingjubilee avatar Jul 02 '25 06:07 workingjubilee