Computed relationships don't support Nested Embedding #2530

yyqgituser · 2022-10-26T07:38:48Z

Environment

PostgreSQL version: 12.11
PostgREST version: v10.0.0

Description of issue

I am trying to upgrade PostgREST from version 6.0.2 to latest 10.0.0, as mentioned in the release note for 10.0.0, thre is a breaking change:

Many-to-many relationships now require that foreign key columns be part of the join table composite key

we have tables which can't fullfill this requiremnt and it is not easy to fix by changing tables, so I try to solve it by Computed relationships. consider following example database:

SQL script

create table actors (
  id integer,
  name text,
  primary key (id)
);
create table directors (
  id integer,
  name text,
  primary key (id)
);
create table films (
  id integer,
  director_id integer references directors(id),
  title text,
  primary key (id)
);
create table roles (
  film_id integer references films(id),
  actor_id integer references actors(id),
  primary key(film_id, actor_id) -- delete this line to remove primary key
);

create function film_to_actor_map(films) returns setof actors as $$
  select a.* from roles as r
  	left join actors as a on r.actor_id = a.id
  where r.film_id = $1.id
$$ stable language sql;

create function actor_to_film_map(actors) returns setof films as $$
  select f.* from roles as r
  	left join films as f on r.film_id = f.id
  where r.actor_id = $1.id
$$ stable language sql;

insert into directors (id, name) VALUES (1, 'd1');
insert into directors (id, name) VALUES (2, 'd2');
insert into actors (id, name) VALUES (1, 'a1');
insert into actors (id, name) VALUES (2, 'a2');
insert into actors (id, name) VALUES (3, 'a3');
insert into films (id, director_id, title) VALUES (1, 1, 'f1');
insert into films (id, director_id, title) VALUES (2, 1, 'f2');
insert into films (id, director_id, title) VALUES (3, 2, 'f3');
insert into roles (film_id, actor_id) VALUES (1, 1);
insert into roles (film_id, actor_id) VALUES (1, 2);
insert into roles (film_id, actor_id) VALUES (2, 2);
insert into roles (film_id, actor_id) VALUES (3, 3);

Here I implements a many-to-many relationship between films and actors by two means, both following request can get same correctly result:

Response

[
    {
        "id": 1,
        "name": "a1",
        "films": [
            {
                "id": 1,
                "director_id": 1,
                "title": "f1"
            }
        ]
    },
    {
        "id": 2,
        "name": "a2",
        "films": [
            {
                "id": 1,
                "director_id": 1,
                "title": "f1"
            },
            {
                "id": 2,
                "director_id": 1,
                "title": "f2"
            }
        ]
    },
    {
        "id": 3,
        "name": "a3",
        "films": [
            {
                "id": 3,
                "director_id": 2,
                "title": "f3"
            }
        ]
    }
]

However it seems that Computed relationships can't support Nested Embedding:
http://localhost:3000/actors?select=id,name,films:actor_to_film_map(id,directors(id,name),title)
404
{
"code": "42P01",
"details": null,
"hint": null,
"message": "missing FROM-clause entry for table "films""
}

Without Computed relationships, everything works correctly
http://localhost:3000/actors?select=id,name,films(id,directors(id,name),title)

Expected Response

[
    {
        "id": 1,
        "name": "a1",
        "films": [
            {
                "id": 1,
                "title": "f1",
                "directors": {
                    "id": 1,
                    "name": "d1"
                }
            }
        ]
    },
    {
        "id": 2,
        "name": "a2",
        "films": [
            {
                "id": 1,
                "title": "f1",
                "directors": {
                    "id": 1,
                    "name": "d1"
                }
            },
            {
                "id": 2,
                "title": "f2",
                "directors": {
                    "id": 1,
                    "name": "d1"
                }
            }
        ]
    },
    {
        "id": 3,
        "name": "a3",
        "films": [
            {
                "id": 3,
                "title": "f3",
                "directors": {
                    "id": 2,
                    "name": "d2"
                }
            }
        ]
    }
]

So is this the limiation of Computed relationships? is there any workaround for it?
Thanks.

wolfgangwalther · 2022-10-26T08:03:31Z

This was already reported in #2455 and fixed in #2519. Not released, yet. You can expect a new pre-release in the next couple of days (currently fixing a few other bugs as well) and probably a patch release soon after. @steve-chavez WDYT?

steve-chavez · 2022-10-26T17:58:02Z

Yeah, a patch release sounds good!

steve-chavez · 2022-10-27T01:41:38Z

@wolfgangwalther There's a problem with the patch release. See the latest prelease notes, the db-pool-timeout was removed which would be a breaking change.

On nikita-volkov/hasql-pool#22 seems they're thinking to restore the timeout.

I guess we have these options for a patch release:

restore the db-pool-timeout and do nothing with it.
wait or contribute to hasql-pool to get back the timeout and restore db-pool-timeout.
Temporarily revert the hasql-pool version upgrade Upgrade hasql-pool to forked 0.7.2 #2444. After the patch release bring it back.

1 would be misleading, 2 might take longer, I think we should do 3. WDYT?

(I must say #2444 is critical for heavy traffic, it's a great improvement.)

wolfgangwalther · 2022-10-27T13:46:01Z

On nikita-volkov/hasql-pool#22 seems they're thinking to restore the timeout.

I think you misread that thread. It's about forcing the acquisition timeout, but there's nothing about introducing the idle timeout again.

I don't think 2 is an option, I don't see the idle timeout coming back. This has been taken out on purpose for some major simplification of hasql-pool.

There's two things that are possibly breaking about removing the db-pool-timeout:

The config option is removed and could throw errors when using it.
The behavior is changed, so that idle connections are not closed.

While there is nothing that we can do about the second point, it's also nothing that stops people from upgrading. So I'm tempted to mark that as a "change" instead of a "breaking change". That means the next release would not be a patch release, but a minor release would be ok. We need that anyway, because db-pool-acquisition-timeout is a new feature.

That is as long as keeping the config option in a config file, environment variable or database setting will not break anything. But that should be fine, I'll just add some tests to confirm that non-existing config options don't break.

steve-chavez · 2022-10-28T03:42:17Z

I think you misread that thread. It's about forcing the acquisition timeout, but there's nothing about introducing the idle timeout again.

Aargh 🤦‍♂️, yes you're right. Many thanks for clarifying that 🙏

On a separate note, we should have a default for the acquisition timeout. Under heavy traffic I've been seeing a similar effect to #2490 and the timeout makes it a constant CPU usage. Will open a PR for this.

yyqgituser · 2022-10-31T03:18:13Z

I have verfied the offical release today, many thanks for your quick support.

wolfgangwalther closed this as completed Oct 26, 2022

wolfgangwalther mentioned this issue Oct 27, 2022

Test non existing config option / clean up CHANGELOG #2539

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Computed relationships don't support Nested Embedding #2530

Computed relationships don't support Nested Embedding #2530

yyqgituser commented Oct 26, 2022 •

edited by wolfgangwalther

Loading

wolfgangwalther commented Oct 26, 2022

steve-chavez commented Oct 26, 2022

steve-chavez commented Oct 27, 2022 •

edited

Loading

wolfgangwalther commented Oct 27, 2022

steve-chavez commented Oct 28, 2022

yyqgituser commented Oct 31, 2022

Computed relationships don't support Nested Embedding #2530

Computed relationships don't support Nested Embedding #2530

Comments

yyqgituser commented Oct 26, 2022 • edited by wolfgangwalther Loading

Environment

Description of issue

wolfgangwalther commented Oct 26, 2022

steve-chavez commented Oct 26, 2022

steve-chavez commented Oct 27, 2022 • edited Loading

wolfgangwalther commented Oct 27, 2022

steve-chavez commented Oct 28, 2022

yyqgituser commented Oct 31, 2022

yyqgituser commented Oct 26, 2022 •

edited by wolfgangwalther

Loading

steve-chavez commented Oct 27, 2022 •

edited

Loading