Postgraphile computed column fails with tsrange and additional parameters

[MartinPELCAT]

Postgraphile computed column fails with tsrange and additional parameters

[benjie]

This is now fixed on main; plus I've fixed the optimization issue that meant that this wasn't just inlined in the first place.

Please note that this next release will be a big one, including the work on #2013 (which I was meant to be working on today 😅), so it's unlikely to go out in the next couple of weeks.

[benjie]

More complete schema:

drop schema if exists space cascade;
create schema space;
CREATE TYPE space.pad_type AS ENUM('MOBILE', 'STATIC', 'TEMPORARY');
CREATE TYPE space.launch_pad AS (id BIGINT, type space.pad_type);

-- DROP TABLE IF EXISTS space.mobile_pad;
CREATE TABLE space.mobile_pad (
    id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
    "name" TEXT NOT NULL,
    time_to_reach_in_hours NUMERIC DEFAULT 10
);

-- DROP TABLE IF EXISTS space.static_pad;
CREATE TABLE space.static_pad (
    id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
    "name" TEXT NOT NULL,
    time_to_reach_in_hours NUMERIC DEFAULT 5
);

-- DROP TABLE IF EXISTS space.temp_pad;
create TABLE space.temp_pad (
    id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
    "name" TEXT NOT NULL,
    time_to_reach_in_hours NUMERIC DEFAULT 15
);

-- DROP TABLE IF EXISTS space.spacecraft;
CREATE TABLE IF NOT EXISTS space.spacecraft (
    id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
    "name" TEXT NOT NULL,
    return_to_earth TSRANGE NOT NULL
);


-- DROP FUNCTION space.spacecraft_eta;
CREATE OR REPLACE FUNCTION space.spacecraft_eta(
    spacecraft space.spacecraft,
    "to" space.launch_pad
) RETURNS TSRANGE LANGUAGE plpgsql STABLE 
AS $$
BEGIN
    RETURN $1.return_to_earth;
END $$;

insert into space.mobile_pad(name) select i::text from generate_series(1, 10) i;
insert into space.static_pad(name) select i::text from generate_series(1, 10) i;
insert into space.temp_pad(name) select i::text from generate_series(1, 10) i;
insert into space.spacecraft(name, return_to_earth) select i::text, tsrange((date_trunc('day', now()) - (i+1) * interval '1 day')::timestamp, (date_trunc('day', now()) - (i) * interval '1 day')::timestamp, '[)') from generate_series(1, 10) i;

[benjie]

Well ain't this just a spanner in the works 😭

[MartinPELCAT]

Thank you for your reactivity, that was way faster than I could've imagined 👌

[benjie]

It seems that we're calling this database function separately rather than inlining it, for some reason. Reason unknown at this time, but it's somewhat irrelevant to the issue (although if we were inlining then it wouldn't be an issue in this case...)

So we're pulling down the record as a string, and then we're using makeSQLValueToRecord(attributes)(recordString) to parse this record string... But that then uses the codec.fromPg function to parse the tsrange attribute, which relies on castFromPg to have added casts when fetching the record... but there are no casts because this is just a plain record text. So effectively it tries to read [TS,TS) as if it were [true, "TS", "TS", false] (JSON) and fails.

So... The whole castFromPg system is problematic. Effectively we need a fromPgWithoutCasting for every codec that has castFromPg (for those that don't, fromPg will be the same with/without casting because there is no casting). But if we have fromPgWithoutCasting anyway then what's the point in casting in the first place?

Alternatively, the way that we do table::text is wrong, and we should instead build a cast expression to use instead. But that's a somewhat involved alternative that will significantly complicate the generated SQL.

[benjie]

My gut says we should just remove castFromPg entirely and use the codecs for parsing without any casting. But that may complicate the common case.

An alternative might be to require fromPgWithoutCasting when castFromPg is present, and to just throw an error if this function is ever needed and isn't present ("sorry, codec doesn't support casting from raw text at this time").

And of course figuring out why this function isn't being inlined in the first place would also help.

[benjie]

Refusing to optimise PgSelect{3}<spacecraft>[28] due to dependency Object<{id,type}>[26]