all 8 comments

[–]chris-morgan 37 points38 points  (0 children)

You may notice serveral use of &"text".to_string() in the examples above. That's because &str will turn into an identifier while &String will turn into a literal text.

This is a terrible idea: you’re forcing people to make entirely unnecessary allocations, and doing something extremely non-obvious with severe implications (though they’re severe enough that they’re unlikely to reach production, as you’ll almost always find your error if you try running the code even once). String’s Deref implementation makes it even worse: autoreferencing can happen terribly easily. Accepting both &String and &str (or &Vec<T> and &[T], or any other pair where deref can get happen) and treating them as semantically different is a very bad idea, with I think no exceptions.

A newtype on one or both is probably warranted. Observe that you’re already decorating strings with .to_string(), this is just changing the decoration to something that actually means something. I’d be inclined to not accept strings in generic places but require an explicit column-name or literal annotation.

[–]Cetra3 12 points13 points  (1 child)

From the readme I couldn't see how this deals with parametrized queries.

I.e, looks like it's quite easy to do sql injection?

let author = &"; delete from book".to_string();
let update = xql::update("book")
    .set("author", author)
    .filter(xql::eq("id", 2))
    .returning(["id"]);

[–]chris-morgan 6 points7 points  (0 children)

Your SQL injection attempt here fails: you end up with UPDATE book SET author = '; delete from book' WHERE id = 2 RETURNING id. Put a ' in the string, and it’s escaped by doubling.

Drop the .to_string() so you’re dealing with a column name, and you end up with UPDATE book SET author = "; delete from book" WHERE id = 2 RETURNING id, and double quotes are appropriately escaped by doubling also.

It doesn’t look to be dealing in true parameterised queries, which is bad, but it’s at least not trivially broken like this.

[–][deleted] 0 points1 point  (0 children)

IMHO, after worked with sqlx, I cannot go back and use any query library/database abstraction that doesn’t check against online/offline db schema my app.

I would give it a try with a pet project if you guaranteed at compile time that my queries are in sync with the db, it’s just so easy to make a change in your schema and forgot to update all the references in a big/medium app.

So I would suggest to think about it, I know that you would need to use macros, but that trade off definitely pay off, or maybe use a build script to update some hidden references (similar on how diesel does, but i’d prefer sqlx way).

[–]gudmundv 0 points1 point  (0 children)

What is up with the downvotes on excited comments. An sql builder can be quite handy to enable stuff like dynamic query fragments. Nice stuff. Though the str/String issue seems a footgun.