Wrong select parameter generated by deleteByPrimaryKey with customEndpoint

[tad-lispy]

Hi! Thanks a lot for this package. It's awesome. To the point:

I have an endpoint with the following definiton:

collections :  Postgrest.Endpoint Collection
collections =
    let
        selectFromDeck =
            [ "id"
            , "name"
            , "content->title"
            ]
                |> Postgrest.attributes
                |> Postgrest.resource "decks"
    in
    Postgrest.customEndpoint
        "collections"
        collectionDecoder
        { defaultSelect =
            Postgrest.allAttributes
                |> (::) selectFromDeck
                |> Just
        , defaultOrder = Nothing
        }

As you can see it gets collections and associated decks. It works correctly except for the deleteByPrimaryKey, like here:

deleteCollection : (Result Postgrest.Error CollectionId -> msg) -> Postgrest.JWT -> CollectionId -> Cmd msg
deleteCollection callback token id =
    id
        |> Postgrest.deleteByPrimaryKey collections collectionKey
        |> Postgrest.toCmd token callback

This produces following HTTP request:

http://[...]/collections?id=eq.780e27ac-7834-468e-978a-7e097254af52&limit=1&select=decks(id,name,content-%3Etitle),*

Please note the select query parameter. In my case this results in a 400 response with the following body:

{
    "hint":null,
    "details":null,
    "code":"42702",
    "message":"column reference \"id\" is ambiguous"
}

The following is a workaround:

  deleteCollection : (Result Postgrest.Error CollectionId -> msg) -> Postgrest.JWT -> CollectionId -> Cmd msg
  deleteCollection callback token id =
      id
          |> Postgrest.deleteByPrimaryKey collections collectionKey
+         |> Postgrest.setParams [ [ "id" ] |> Postgrest.attributes |> Postgrest.select ]
          |> Postgrest.toCmd token callback

Without looking much into it I would guess that the deleteByPrimaryKey should override the defaultSelect or maybe even the select param and only select the primary key.

The error from PostgreSQL server

``` 2020-08-21 21:08:05.374 UTC [1919] ERROR: column reference "id" is ambiguous at character 771 2020-08-21 21:08:05.374 UTC [1919] STATEMENT: WITH pgrst_source AS (WITH pgrst_ignored_body AS (SELECT $1::text) DELETE FROM "api"."collections" WHERE "api"."collections"."id" = '780e27ac-7834-468e-978a-7e097254af52'::unknown RETURNING "api"."collections".*, "api"."collections"."id") SELECT '' AS total_result_set, pg_catalog.count(_postgrest_t) AS page_total, array[]::text[] AS header, coalesce(json_agg(_postgrest_t), '[]')::character varying AS body, coalesce(nullif(current_setting('response.headers', true), ''), '[]') AS response_headers FROM (SELECT "pgrst_source".*, COALESCE ((SELECT json_agg("pgrst_source".*) FROM (SELECT "api"."decks"."id", "api"."decks"."name", "api"."decks"."content"->'title' AS "title" FROM "api"."decks" WHERE "pgrst_source"."id" = "api"."decks"."collection" ) "pgrst_source"), '[]') AS "decks" FROM "pgrst_source" LIMIT 1 OFFSET 0) _postgrest_t 2020-08-21 21:08:05.374 UTC [1919] ERROR: column reference "id" is ambiguous at character 771 2020-08-21 21:08:05.374 UTC [1919] STATEMENT: WITH pgrst_source AS (WITH pgrst_ignored_body AS (SELECT $1::text) DELETE FROM "api"."collections" WHERE "api"."collections"."id" = '780e27ac-7834-468e-978a-7e097254af52'::unknown RETURNING "api"."collections".*, "api"."collections"."id") SELECT '' AS total_result_set, pg_catalog.count(_postgrest_t) AS page_total, array[]::text[] AS header, coalesce(json_agg(_postgrest_t), '[]')::character varying AS body, coalesce(nullif(current_setting('response.headers', true), ''), '[]') AS response_headers FROM (SELECT "pgrst_source".*, COALESCE ((SELECT json_agg("pgrst_source".*) FROM (SELECT "api"."decks"."id", "api"."decks"."name", "api"."decks"."content"->'title' AS "title" FROM "api"."decks" WHERE "pgrst_source"."id" = "api"."decks"."collection" ) "pgrst_source"), '[]') AS "decks" FROM "pgrst_source" LIMIT 1 OFFSET 0) 2020-08-21 21:08:05.374 UTC [1919] ERROR: column reference "id" is ambiguous at character 771 2020-08-21 21:08:05.374 UTC [1919] STATEMENT: WITH pgrst_source AS (WITH pgrst_ignored_body AS (SELECT $1::text) DELETE FROM "api"."collections" WHERE "api"."collections"."id" = '780e27ac-7834-468e-978a-7e097254af52'::unknown RETURNING "api"."collections".*, "api"."collections"."id") SELECT '' AS total_result_set, pg_catalog.count(_postgrest_t) AS page_total, array[]::text[] AS header, coalesce(json_agg(_postgrest_t), '[]')::character varying AS body, coalesce(nullif(current_setting('response.headers', true), ''), '[]') AS response_headers FROM (SELECT "pgrst_source".*, COALESCE ((SELECT json_agg("pgrst_source".*) FROM (SELECT "api"."decks"."id", "api"."decks"."name", "api"."decks"."content"->'title' AS "title" FROM "api"."decks" WHERE "pgrst_source"."id" = "api"."decks"."collection" ) "pgrst_source"), '[]') AS "decks" FROM "pgrst_source" LIMIT 1 OFFSET 0) 2020-08-21 21:08:05.374 UTC [1919] ERROR: column reference "id" is ambiguous at character 771 2020-08-21 21:08:05.374 UTC [1919] STATEMENT: WITH pgrst_source AS (WITH pgrst_ignored_body AS (SELECT $1::text) DELETE FROM "api"."collections" WHERE "api"."collections"."id" = '780e27ac-7834-468e-978a-7e097254af52'::unknown RETURNING "api"."collections".*, "api"."collections"."id") SELECT '' AS total_result_set, pg_catalog.count(_postgrest_t) AS page_total, array[]::text[] AS header, coalesce(json_agg(_postgrest_t), '[]')::character varying AS body, coalesce(nullif(current_setting('response.headers', true), ''), '[]') AS response_headers FROM (SELECT "pgrst_source".*, COALESCE ((SELECT json_agg("pgrst_source".*) FROM (SELECT "api"."decks"."id", "api"."decks"."name", "api"."decks"."content"->'title' AS "title" FROM "api"."decks" WHERE "pgrst_source"."id" = "api"."decks"."collection" ) "pgrst_source"), '[]') AS "decks" FROM "pgrst_source" LIMIT 1 OFFSET 0) _postgrest_t ```

[alex-tan]

Thanks for the bug report @tad-lispy.

Could you provide your postgrest/postgres versions? Also is collections a table or a view? If it's a view could you provide the SQL for it?

I'm surprised the request is failing because I believe postgrest allows you to return the deleted entity, although perhaps the issue is with the nested resource.

[tad-lispy]

Hey! Thanks for getting back to me.

Yes, I guess it's because of the nested resource. Potentially related issue: https://github.com/PostgREST/postgrest/issues/1576.

I'm on latest stable: PostgREST 7.0.1 and Postgres 12.4.

Both collections and decks are views in the api schema becked by private schema containing tables. I believe the relevant SQL at the time was like this:

Create table if not exists private.collections (
    id uuid primary key default uuid_generate_v4(),
    owner uuid not null references private.teachers(id) default current_setting('request.jwt.claim.teacher')::uuid,
    name citext not null unique,
    title text not null,

    constraint filename check (name ~* '^[a-z0-9\-\_\.\(\)]+$')
);

Create table if not exists private.decks (
    id uuid primary key default uuid_generate_v4(),
    collection uuid not null references private.collections(id),
    name citext not null,
    content jsonb not null,

    constraint filename check (name ~* '^[a-z0-9\-\_\.\(\)]+$')
);
Create unique index collection_file on private.decks (
    name,
    collection
);

Create view api.collections as
select
    collections.id,
    collections.owner,
    collections.name,
    collections.title
from
    private.collections
;

Create or replace function delete_collection_function()
returns trigger as
$$
<<variables>>
Declare
    owner uuid;
Begin
    Select private.collections.owner
    from private.collections
    where name = old.name
    into variables.owner;

    If variables.owner is null
    then
        raise sqlstate 'PT404'
        using
            message = 'This collection is not registered',
            detail = old.name,
            hint = 'Cannot remove a non-existent collection. A typo?'
        ;
    end if;

    If not variables.owner = current_setting('request.jwt.claim.teacher')::uuid
    then
        raise insufficient_privilege
        using
            hint = 'You are not the owner of this collection',
            detail = old.name
        ;
    end if;

    Delete from private.collections
    where private.collections.name = old.name;

    return null;
end;
$$
language plpgsql
security definer;

Create trigger delete_collection_trigger
instead of delete on api.collections
for each row
execute function delete_collection_function();

Create view api.decks as
select
    decks.id,
    decks.collection,
    decks.name,
    decks.content
from
    private.decks
;

One controversial design decision I made early and abandoned since was to name all primary keys id (e.g. collections.id, decks.id etc). That may have contribute to the confusion. So since I posted the issue I changed that and now all keys are prefixed, like collections.collection_id, decks.deck_id, etc). But I believe it had no effect on the issue.