Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for negating Condition #147

Closed
nicoulaj opened this issue Sep 10, 2021 · 6 comments
Closed

Add support for negating Condition #147

nicoulaj opened this issue Sep 10, 2021 · 6 comments

Comments

@nicoulaj
Copy link

It is tempting to use Condition to build APIs that accept composite filters. However, once a Condition has been built, there is no way to introspect it (fields are private), and no way to negate it (NOT operator is exposed on Expr instead).

It would be nice to have either:

  • a Condition::not(&self) method
  • a public Visitor or similar API so that Condition can be introspected
@nicoulaj nicoulaj changed the title Add support negating Condition Add support for negating Condition Sep 10, 2021
@billy1624
Copy link
Member

Related to SeaQL/sea-query#132

@tyt2y3 tyt2y3 added this to the 0.3.x milestone Sep 26, 2021
@6xzo
Copy link

6xzo commented Sep 28, 2021

I have implemented something for this, and it turned out a bit different than I expected going into it. I wound up with new methods (and macros) Condition::not_any() -> Self and Condition::not_all() -> Self. I also added Condition::not(mut self) -> Self, which turns an any or all into the negative version. The implementation is simply a new boolean field in Condition and emission of "NOT", with parens in the case of multiple terms, in QueryBuilder::prepare_condition_where.

The unexpected outcome for me was the idea of negating a conjunction or disjunction, instead of "not" being a unary operator.

any      [ T x x ... ]  OR
not_any  [ F F F ... ]  NOT OR
all      [ T T T ... ]  AND
not_all  [ F x x ... ]  NOT AND

I am concerned that the not method is confusing. Should it turn a not_any into an any? Should multiple applications of it keep inverting the sense? The API might be most clear without it.

If this suits, I could prepare a PR.

@6xzo
Copy link

6xzo commented Sep 28, 2021

In actually trying to use my approach to solve the problem that I described here (SeaQL/sea-query#143), I discovered that maybe I'm in over my head... The problem is, wrapping a complex condition into a new layer of not_all still does not emit the necessary parentheses...

use sea_query::*;

fn main() {
    let t1 = Expr::col(Alias::new("a")).eq(1);
    let t2 = Expr::col(Alias::new("b")).eq(1);
    let expr = Condition::all()
        .add(t1)
        .add(t2);
    let expr = Condition::not_all().add(expr);
    // let expr = expr.not();
    let query = Query::select()
        .cond_where(expr)
        .to_string(MysqlQueryBuilder);
    println!("{}", query);
}

prints

SELECT WHERE NOT `a` = 1 AND `b` = 1

Using the commented out line instead of the line above it does emit parentheses.
The problem is, I don't understand the design of Condition/prepare_condition_where well enough to think that I can design a good solution to this. It is insufficient to look at one layer of condition.condition.len() > 1.

@tyt2y3
Copy link
Member

tyt2y3 commented Sep 28, 2021

Thank you for the effort. I think a 'not()' method is good enough.

And 'not_all()' is really only a shorthand for 'all().not()' right? There is no need to have an extra operator 'not_all'.

That would pretty much do it! I would appreciate to have a look over the PR.

@tyt2y3 tyt2y3 modified the milestones: 0.3.x, 0.2.x Sep 29, 2021
@tyt2y3
Copy link
Member

tyt2y3 commented Sep 29, 2021

Seemingly we can ship this in 0.2 after bumping SeaQuery, cheers

@tyt2y3
Copy link
Member

tyt2y3 commented Oct 1, 2021

0.2.4

@tyt2y3 tyt2y3 closed this as completed Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants