You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Remove the HasSqlType bound everywhere it's not required
Back in 0.4, this was a trait called `NativeSqlType`. I stuck it as a
bound on virtually every place a SQL type was expected, since it served
as nice documentation for "the set of types you can use here". It lived
on things like `Query` and `Expression` as well.
In 0.5 we added support for SQLite, and things got considerably more
complex. This trait could no longer appear in places that the backend
doesn't, and the "implementors" section is much more noisy and no longer
useful documentation of "what are the SQL types".
Ultimately this trait has a single concrete use, and that is to get the
metadata associated with bind parameters. So `BindCollector` needs it,
`AstPass` needs it on the methods that interact there, and `Bound` will
need it in its `QueryFragment` impl. Virtually every other place that
has that bound doesn't need it, and requiring it usually just means "oh
I have to stick this additional useless bound here that I don't know or
understand". Really the annoyance that made me want this is that you
usually wouldn't even import the trait otherwise.
One point to note here is that I was unable to remove this bound from
the `Connection` trait. This is because the MySQL connection is actually
using this bound to tell the C API what type we expect the results to
be. If I change the code to do the same thing we do for `sql_query`,
where we simply ask the database for the result types, we learn that
MySQL does a lot of things differently than we expected:
- 32 bit addition/subtraction returns a 64 bit integer
- 32 bit multiplication/division returns numeric
- 32 bit sum returns numeric
- 32 bit bind parameters are converted to 64 bit
So it turns out that we're relying on the client library to do implicit
coercion in more places than we should be. Ultimately we're going to
need to start doing those conversions ourselves if we ever want to dump
the C API. Once we start doing that, we can remove `HasSqlType` from
`Connection` (and thus also from `LoadQuery`), and then we can deprecate
the `row_metadata` method from that trait.
0 commit comments