sfackler / rust-postgres

Native PostgreSQL driver for the Rust programming language
Apache License 2.0
3.52k stars 446 forks source link

Avoid extra clone in config if possible #1137

Closed nyurik closed 4 months ago

nyurik commented 7 months ago

Using impl Into<String> instead of &str in a fn arg allows both &str and String as parameters - thus if the caller already has a String object that it doesn't need, it can pass it in without extra cloning.

The same might be done with the password, but may require closer look.

sfackler commented 7 months ago

I'm not super opposed to this, but at the same time I'm not sure it'll make a huge difference. What portion of runtime is spent constructing config?

nyurik commented 7 months ago

@sfackler config specifically is not that big of a deal of course. My concern is that any frequently used code teaches users on how to do things - and in this case, IMO, the API should have only allowed String as the parameter, but because breaking API is not worth it, might as well introduce a backwards compatible solution.

Reasoning:

P.S. community recommended using explicit generic rather than impl Into<String> as a param type.