Feature flag to have more parentheses

[tyt2y3]

@Sytten

This is what I came up with: feature flag + runtime option. It has a safety mechanism to prevent multiple code paths overriding the same option twice.

Let me know if it works for you

[Sytten]

Interesting choice, why the runtime option? I think a compilation option is enough? EDIT: Saw the comment regarding serde. I get the concern. A better approach would be to pass down the option to the query builder but it would be a bigger change I guess. Not a fan of globals atomic but I can live with them if that's the best option.

[tyt2y3]

A better approach would be to pass down the option to the query builder but it would be a bigger change

Yes I considered that, but it's a bigger change to user code as well. If we add a new builder API (build_with_option(o: Option)) that accepts a config object, you'd need to pass it around everywhere. Not sure if you'd prefer that?

[Sytten]

Only if we think we will need other options in the future. My ask is quite niche so I would not do it just for it. I am just not a fan of the atomic runtime, but it's not the end of the world. That is why I preferred a compile time option.

[tyt2y3]

Okay, I'll remove the global state for now. Oh, I hit a problem. If we add a prefer-more-parentheses feature flag, it'll be turned on unexpectedly by --all-features, which will be confusing, especially if sea-query is not a direct dependency.

[Sytten]

I don't think thats used really often, most crates have conflicting features anyway (async runtimes for example). I would do a naming scheme for them, something like: option-more-parentheses

[tyt2y3]

@Sytten I'd invite you to may be add a more complicated test case to tests/more-parentheses.rs to make sure you get what you need.

[Sytten]

#[test]
fn test_more_parentheses_complex() {
    // Add pagination
    let mut pagination = Cond::all();
    let lt_value = Expr::col(Glyph::Aspect)
        .lt(1)
        .or(Expr::col(Glyph::Aspect).is_null());
    let lt_id = Expr::col(Glyph::Aspect)
        .is(1)
        .and(Expr::col(Glyph::Id).lt(10));
    pagination = pagination.add(lt_value.or(lt_id));

    // Add filtering
    let mut all = Cond::all();
    all = all.add(Expr::col(Glyph::Image).eq("png"));

    let mut nested = Cond::all();
    nested = nested.add(Expr::col(Glyph::Table).gte(5));
    nested = nested.add(Expr::col(Glyph::Tokens).lte(3));
    all = all.add(nested);

    let mut any = Cond::any();
    any = any.add(Expr::col(Glyph::Image).like("%.jpg"));
    any = any.add(all);
    let filtering = any;

    // Query
    let query = Query::select()
        .column(Glyph::Id)
        .from(Glyph::Table)
        .cond_where(Cond::all())
        .cond_where(pagination)
        .cond_where(filtering)
        .to_owned();

    assert_eq!(
        query.to_string(MysqlQueryBuilder),
        "SELECT `id` FROM `glyph` WHERE ((`aspect` < 1) OR (`aspect` IS NULL) OR ((`aspect` IS 1) AND (`id` < 10))) AND ((`image` LIKE '%.jpg') OR ((`image` = 'png') AND ((`glyph` >= 5) AND (`tokens` <= 3))))"
    );
}

Done @tyt2y3

[github-actions[bot]]

:tada: Released In 0.30.5 :tada:

Thank you everyone for the contribution! This feature is now available in the latest release. Now is a good time to upgrade! Your participation is what makes us unique; your adoption is what drives us forward. You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.