Codecov Report

Merging #908 (cfd351c) into master (2c64275) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #908   +/-   ##
=======================================
  Coverage   77.35%   77.35%           
=======================================
  Files          48       48           
  Lines        5600     5600           
=======================================
  Hits         4332     4332           
  Misses       1268     1268           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c64275...acac5a1. Read the comment docs.

Is it the same issue as described here:
#312 (comment)
?

Is it the same issue as described here:
#312 (comment)
?

It's similar, but with named params only.

Anyway, I'm not sure it's worth it, although there's no real alternative if you did have a Vec<(&str, Box<dyn ToSql>)>.

That said, this fix doesn't fix it if you had a Vec<(String, Box<dyn ToSql>)>. Maybe it could, but I'm worried about accidentally adding another case where something that seems like it should typecheck fails (similar to NO_PARAMS being necessary since &[] is ambiguous)

Maybe handling it the way iterables are handled would be better...

Maybe handling it the way iterables are handled would be better...

You mean like ParamsFromIter ?

pub struct NamedParamsFromIter<I>(I);

pub fn named_params_from_iter<I>(iter: I) -> ParamsFromIter<I>
where
    I: IntoIterator,
    I::Item: (&str, ToSql), // or T: ToSql
{
    NamedParamsFromIter(iter)
}

Basically. I think we could solve the &str vs String case too like:

pub struct NamedParamsFromIter<I>(I);

pub fn named_params_from_iter<I, S, T>(iter: I) -> NamedParamsFromIter<I>
where
    I: IntoIterator<Item = (S, T)>,
    S: AsRef<str>,
    T: ToSql,
{
    NamedParamsFromIter(iter)
}

(Haven't actually tried this code)

The use case here seems pretty niche still though, but it's not like there's a good way to do it otherwise, and this shouldn't negatively impact normal usage. Hrm.

Do you know if something like BorrowToSql would help here ?

A trait used by clients to abstract over &dyn ToSql and T: ToSql

@thomcc Less that I don't "need" it as much as it is possible to work around by using numbered parameters. The ability to do this would still be very much appreciated if it doesn't end up breaking other functionality.

@thomcc are you thinking of finishing this PR? This looks exactly like something I was looking for.

I'll look into finishing it this weekend.

See #1652 with BindIndex