Postgraphile computed column fails with tsrange and additional parameters

[MartinPELCAT]

Summary

When using the computed columns there is a strange behavior with the additional parameters and the type tsrange from postgres.
Note that the errors do not show when testing the functions directly against the database but it only appears when running the graphql queries.

The following code snippets comes from the migrations in the flyway folder.

In this code snippet, you can see that we have a custom type to space.launch_pad as a second parameter. Even if it's not used it's not the problem here.

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

When running the following query, you can see that the eta column will be incorrect.
Note that it's note the same error we face depending on the tsrange value.

A value like: ["2024-03-21 00:00:00","2024-03-22 23:59:59"] where [] would take the upper bound and set is as the lower bound of the result
A value like: ["2024-03-20 00:00:00","2024-03-23 00:00:00") where [) would completely fail

query {
  allSpacecrafts {
    nodes {
      id
      name
      eta(to: { id: "1", type: MOBILE }) {
        start {
          value
        }
        end {
          value
        }
      }
    }
  }
}

Running the exact same query without the parameter works fine

query {
  allSpacecrafts {
    nodes {
      id
      name
      eta {
        start {
          value
        }
        end {
          value
        }
      }
    }
  }
}

The probleme seems to appears when using a "custom type" as a parameter.

Steps to reproduce

I've added a complete reproduction repo for this case.

But basically this running this would be enough to reproduce.

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

Expected results

Works as expected

Actual results

A value like: ["2024-03-21 00:00:00","2024-03-22 23:59:59"] where [] would take the upper bound and set is as the lower bound of the result
A value like: ["2024-03-20 00:00:00","2024-03-23 00:00:00") where [) would completely fail

Additional context

https://github.com/MartinPELCAT/postgraphile-debug

[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 😭

[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]

[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.

[MartinPELCAT]

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