Unable to delete item if it is present with same id in the multiple collections

[ivica3730k]

Hi all!
Using a stac-fastapi (pg) I found the following. If the item with the same id is present in the database(in my case item with id LC08_L2SP_202025_20210714_02_T1 being present both in landsat-c2-l2 and landsat-8-c2-l2 collection) the item can't be deleted
from any of the collections.

Opening the database I can see the following.
I am attaching screenshots below:

My best guess would be that the delete SQL statement needs to be filtered with the collection id.

Thanks!
Ivica

[ivica3730k]

Just confirmed it with Joplin

[geospatial-jeff]

I believe this is somehow related to #381

[ivica3730k]

Thank you very much @geospatial-jeff .

I went over to pgstac github to inspect their stored procedure and it is indeed taking both item_id and collection_id, and the
python code is calling it with single first argument.

https://github.com/stac-utils/pgstac/blob/main/pypgstac/pypgstac/migrations/pgstac.0.6.6.sql

CREATE OR REPLACE FUNCTION delete_item(_id text, _collection text DEFAULT NULL) RETURNS VOID AS $$
DECLARE
out items%ROWTYPE;
BEGIN
    DELETE FROM items WHERE id = _id AND (_collection IS NULL OR collection=_collection) RETURNING * INTO STRICT out;
END;
$$ LANGUAGE PLPGSQL;

I have then edited the dbfunc to also take additional tuple argument

async def dbfunc(pool: pool, func: str, arg: Union[str, Dict,tuple]) -> Dict:
    """Wrap PLPGSQL Functions.

    Keyword arguments:
    pool -- the asyncpg pool to use to connect to the database
    func -- the name of the PostgreSQL function to call
    arg -- the argument to the PostgreSQL function as either a string, tuple of two strings or a dict that will be converted into jsonb
    """
    try:
        if isinstance(arg, str):
            async with pool.acquire() as conn:
                q, p = render(
                    f"""
                        SELECT * FROM {func}(:item::text);
                        """,
                    item=arg,
                )
                return await conn.fetchval(q, *p)
        elif isinstance(arg, tuple):
            async with pool.acquire() as conn:
                q, p = render(
                    f"""
                        SELECT * FROM {func}(:item::text, :collection::text);
                        """,
                    item=arg[0],
                    collection=arg[1],
                )
                return await conn.fetchval(q, *p)
        else:
            async with pool.acquire() as conn:
                q, p = render(
                    f"""
                        SELECT * FROM {func}(:item::text::jsonb);
                        """,
                    item=json.dumps(arg),
                )
                return await conn.fetchval(q, *p)
    except exceptions.UniqueViolationError as e:
        raise ConflictError from e
    except exceptions.NoDataFoundError as e:
        raise NotFoundError from e
    except exceptions.NotNullViolationError as e:
        raise DatabaseError from e
    except exceptions.ForeignKeyViolationError as e:
        raise ForeignKeyError from e

And finally, then I reintroduced collection_id argument in delete_item function in transactions.py

    async def delete_item(
        self, item_id: str, collection_id:str, **kwargs
    ) -> Optional[Union[stac_types.Item, Response]]:
        """Delete item."""
        request = kwargs["request"]
        pool = request.app.state.writepool

        await dbfunc(pool, "delete_item", (item_id, collection_id))
        return JSONResponse({"deleted item": item_id})

This fix seems to work okay to me. Do you think the fix is good enough to proceed to the pull request?

[geospatial-jeff]

I'm not too familiar with pgstac, that is a question for @vincentsarago @bitner @duckontheweb

[ivica3730k]

@vincentsarago @bitner @duckontheweb Hi there. Any thoughts on this fix? Would it be good enough to make a pull request? Any other places where a similar issue could occur(stored procedure not being called with all parameters?)

Ivica