search

diesel-rs / diesel

A safe, extensible ORM and Query Builder for Rust
https://diesel.rs
Apache License 2.0
12.79k stars 1.08k forks source link

Fix and/or function argument to compile check bool expression #4345

Closed E-anae closed 1 week ago

E-anae commented 1 week ago

Currently if you try putting any sort of expression in a and function of diesel it will not be detected at compile time but will raise an error at runtime

Ten0 commented 1 week ago

Thank you for the fix! Tested compilation on our codebase and this does not cause any issue.

@weiznich it looks like we probably want to add this to #4343 :)

weiznich commented 1 week ago

@Ten0 I will add it to the backport PR as soon as I'm to work.

Roardom commented 1 week ago

Hi, maybe I'm misunderstanding, but it looks like this restriction is placed on all database backends. The query provided in the test works fine for mysql on 2.2.4. Maybe it should only restrict postgres instead?

weiznich commented 1 week ago

@Roardom Thanks for raising that concern. You are right that this kind of query is allowed with mysql, it seems to be just another syntax for column IS NOT NULL. As there is currently no easy way to place this restriction as backend specific restriction I would like to move forward anyway as that's a rather critical bugfix for the postgresql/sqlite backend. I'm open to other opinions here, but I would like to ask for proposed solutions then.

Roardom commented 1 week ago

While not a complete solution, it should at least be highlighted in the release notes as a breaking change for users supporting mysql backends. I saw this change and immediately thought of similar queries in my code. But I double checked, and I only use it inside SUM(), such as SUM(NOT active AND visible) which already isn't permitted in diesel (since boolean doesn't implement foldable requiring raw queries).

it seems to be just another syntax for column IS NOT NULL

I believe column IS NOT NULL AND column != 0 AND column != '' is more accurate (0 and '' would depend on the type). There might be other variations as well.