vapor / postgres-nio

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
https://api.vapor.codes/postgresnio/documentation/postgresnio/
MIT License
305 stars 70 forks source link

Add Interpolation Function for Collection types #410

Closed MahdiBM closed 9 months ago

MahdiBM commented 10 months ago

Adds an interpolation function for collection types.

This has proved quite useful since manually interpolating an array is quite hard to do properly, to a PostgresQuery.

codecov-commenter commented 10 months ago

Codecov Report

Merging #410 (62f69a0) into main (92ee156) will decrease coverage by 0.13%. The diff coverage is 77.77%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #410 +/- ## ========================================== - Coverage 49.19% 49.06% -0.13% ========================================== Files 108 108 Lines 8845 8854 +9 ========================================== - Hits 4351 4344 -7 - Misses 4494 4510 +16 ``` | [Files Changed](https://app.codecov.io/gh/vapor/postgres-nio/pull/410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor) | Coverage Δ | | |---|---|---| | [Sources/PostgresNIO/New/PostgresQuery.swift](https://app.codecov.io/gh/vapor/postgres-nio/pull/410?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9Qb3N0Z3Jlc05JTy9OZXcvUG9zdGdyZXNRdWVyeS5zd2lmdA==) | `81.08% <77.77%> (-0.22%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/vapor/postgres-nio/pull/410/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor)
MahdiBM commented 10 months ago

@gwynne sorry i was writing some comments, i knew you'll respond before my comments are done but couldn't do much 😅

MahdiBM commented 10 months ago

I do realize this is tricky and weird, but i also do think this will be a nice addition and hopefully worth the effort.

MahdiBM commented 10 months ago

Screenshot 2023-09-23 at 3 42 33 PM

This test which has started to fail, shows a proper way how to do array interpolation.

@fabianfett @gwynne The approach of this kind of interpolation with no casting looks better and is easier (no need to know the exact postgres type, then type it out etc...), but i honestly hadn't figured out that you could cast a bind to an array type and it would work, so maybe this PR is not anymore of enough value?

I'll remove the underscored attributes and use a parameter name for the collection interpolation instead, for now, until @fabianfett has the time to take a look.

MahdiBM commented 10 months ago

So, I'm testing a little bit and the PR seems to be more useful than I thought might be the case.

Basically, this is the same situation that made me create the interpolation function in a project, and i can't find a way to do it without multiple different binds, which means it cannot be done with current PostgresQuery interpolation functions:

PREPARE my_query (bigint[]) AS SELECT my_column FROM my_table WHERE my_column in ($1);

Execute my_query( (1111, 2222) :: bigint[] );
ERROR: operator does not exist: bigint = bigint[] Hint: No operator matches the given name and argument types. You might need to add explicit type casts.

I've tested a lot of different prepared queries, non worked.

In contrast, what this PR adds:

PREPARE my_query (bigint, bigint) AS SELECT my_column FROM my_table WHERE my_column in ($1, $2);

Execute my_query(1111, 2222);

This executes and returns results as expected. The example query would look like this in PostgresQuery:

"""
SELECT my_column FROM my_table WHERE my_column in \(collection: array_of_values)
"""
MahdiBM commented 9 months ago

After asking some SQL folks, here's the conclusion:

I wanted to do a query like:

SELECT * FROM t WHERE c in $1

This is not possible to do properly without the additions that this PR introduces. Even with this PR, we need to use multiple binds instead of just 1.

It appears the correct way to do that using a single bind is to instead use any:

SELECT * FROM t WHERE c = any($1)

This means that if you think this addition is not of enough value, it's fine with me and I won't even need to leave this code as an extension in my code. I can instead use this alternative way I found with existing PostgresQuery stuff.

I've also tested this with PostgresNIO, and it indeed does behave correctly with any.

Bonus

With any, Postgres users don't even need to worry about their array/collections being empty. Postgres will handle the request in a logical way, and won't throw an error.