Do typechecking on postgres value binder (implement `accepts`)
prakol16 opened this issue · 1 comments
Motivation
If the wrong PostgresValues
parameters are passed to a postgres call (or even if they are the right ones but there is a sea query bug e.g. see #695), the parameters' binary value is simply type cast to the expected types. This makes debugging much more difficult
As an example, consider the following test:
#[test]
fn test_wrong_type_build() {
let mut conn = Client::connect("...", NoTls)
.expect("Should succeed in connecting to postgres db");
let query = Query::select()
.expr(Expr::val("testing text"))
.to_owned();
let query2 = Query::select()
.expr(Expr::val(42))
.to_owned(); // note that query2 wouldn't actually work anyway because of issue #695
let (query, values) = query.build_postgres(PostgresQueryBuilder);
let (query2, values2) = query2.build_postgres(PostgresQueryBuilder);
assert_eq!(query, "SELECT $1");
assert_eq!(query2, "SELECT $1");
let result: String = conn.query_one(&query, &values.as_params())
.expect("regular query should succeed")
.get(0);
assert_eq!(result, "testing text"); // succeeds
let err = conn.query_one(&query, &values2.as_params()) /// OOPS! `values2` passed in with `query` -- they don't match
.expect_err("query with wrong type should fail");
println!("error: {:?}", err);
}
This test succeeds. However, the error it prints out at the end is very difficult to understand:
Error { kind: Db, cause: Some(DbError { severity: "ERROR", parsed_severity: Some(Error), code: SqlState(E22021), message: "invalid byte sequence for encoding \"UTF8\": 0x00", detail: None, hint: None, position: None, where_: Some("unnamed portal parameter $1"), schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("mbutils.c"), line: Some(1665), routine: Some("report_invalid_encoding") }) }
You have to realize that what happened was that Postgres happily tried to cast the values2
parameters to a text value (what it expected). Since 42
has a null character however, it was unable to correctly parse the number 42
as text. This could lead to even more subtle bugs when 42
is swapped for an integer that happens to be interpretable as text.
To ensure type safety, PostgresValue
should implement the accepts
function and explicitly reject with a type error when the expected output is of the wrong type.
Proposed Solutions
The PostgresValue
type should implement the accepts
function of the ToSql
trait (https://docs.rs/postgres/latest/postgres/types/trait.ToSql.html#tymethod.to_sql). Currently, it just returns true
, but this can lead to very subtle bugs where types are reinterpreted as other types with no safety.
Just as an example, this would have made debugging #695 easier as well.
Additional Information
Thank you for the investigation! I didn't know the accepts
function would have such impact.
Would you be able to open a PR on https://github.com/SeaQL/sea-query/blob/master/sea-query-postgres/src/lib.rs ?