SeaQL/sea-query

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 ?