Support "if" and pattern match in where condition

[jindraivanek]

Main motivation for this feature is combining where condition from optional parameters:

select {
    for p in personTable do
    where (
        (match positionGTFilter with | Some x -> p.Position > x | None -> true)
        && (match positionLTFilter with | Some x -> p.Position < x | None -> true)
        && (match firstNameFilter with | Some x -> like p.FirstName x | None -> true))
} |> conn.SelectAsync<Person>

It's adding support for:

• if ... then ... else, where condition needs to be of bool type
• boolean literals (true, false)
• pattern match
  • fun fact: match expression is actually translated to if expressions in Linq.Expression
  • Option, Result and simple custom DU is covered by tests, its quite possible there is more corner cases that's not covered by this
  • to make this work, I needed to add binding substitution to visitWhere (so we are able to replace x with value from positionGTFilter in p.Position > x. It can be used to support lambda functions in other use cases in the future.

✅Whole test suite is green on MyComputer™.

[JordanMarr]

While that is technically very impressive, it seems like a pretty heavy-handed approach for what it accomplishes (conditional where statements) and would add a fair amount of additional complexity.

Have you at least considered a whereIf operation instead? It would be very simple to implement as it can be an extension method that takes a bool condition and then conditionally calls the existing where operation. So simple that it can be added without even modifying the underlying engine.

It has been discussed in the SqlHydra repo (which uses the same engine) here:
JordanMarr/SqlHydra#59

Not to say that it's not worth doing, but maybe worth considering the simpler alternatives. Just my $0.02.

[jindraivanek]

Have you at least considered a whereIf operation instead?

Yes, we was using similar updateWhereIf custom operation, that combine existing where condition with given expression if condition holds. Main problem here is that for using option, you need to use .Value, and its easy to make mistake here:

updateWhereIf And firstNameFilter.IsSome (like p.FirstName firstNameFilter.Value)

I don't see a way how to solve this problem without extending where expression parser.

Another problem is that if you want to combine different operators it's got ugly.

Regarding complexity, IMO it's not much complexity added, main logic of visitWhere remains same, it handles more cases. Agree, that substitution functionality adding some complexity, but IMO its worth it.

I could revisit naming and comments there, if that would be main problem.

[JordanMarr]

Yes, we was using similar updateWhereIf custom operation, that combine existing where condition with given expression if condition holds. Main problem here is that for using option, you need to use .Value, and its easy to make mistake here:

updateWhereIf And firstNameFilter.IsSome (like p.FirstName firstNameFilter.Value)

I don't see a way how to solve this problem without extending where expression parser.

I don't follow why that would be a problem. The where parser should already handle using option values and .Value. (If you are saying that it fails a runtime error, then that further proves my point about complexity).

Another problem is that if you want to combine different operators it's got ugly.

My opinion on the design is that users can safely leverage the entirety of the F# language features outside the Expression parser. By adding more F# syntax tree parsing capabilities inside of the where clause, you run the risk of opening Pandora's box by encouraging users to put more and more free form logic into the Expression tree which will inevitably lead to the discovery of new runtime errors that haven't been handled yet - which then requires further expansion of the parser every time someone posts an issue with what looks to be totally valid F# code inside their where expression and asks, "why doesn't this work?"

On the other hand, keeping it streamlined to a narrow set of supported language features avoids this pitfall.

Regarding complexity, IMO it's not much complexity added, main logic of visitWhere remains same, it handles more cases. Agree, that substitution functionality adding some complexity, but IMO its worth it.
I could revisit naming and comments there, if that would be main problem.

I guess it just depends on how committed you are to the vision of building and maintaining a fully-fledged F# expression parser. It just seems to me that there are simpler ways of achieving conditional where expressions (namely, variants like whereIf).

The SQLProvider codebase provides a very full featured expression parser, and it's really amazing. When I was working on that codebase and admiring the parser, I thought, "nah... I wouldn't want to maintain that." lol!

But maybe if you are volunteering to fix any bugs... 🤔

[JordanMarr]

If you go this route, you should also consider adding a matrix of which F# language features that are supported and which are not within the expressions to the docs so that users have a guideline of what is actually possible without leading to runtime errors.
Otherwise, they will assume it's like SQLProvider and that they can do anything.

[jindraivanek]

I don't follow why that would be a problem. The where parser should already handle using option values and .Value. (If you are saying that it fails a runtime error, then that further proves my point about complexity).

Point is that without this feature, the only way how to construct where from options is by using something like updateWhereIf and .IsSome and .Value, which is error prone and should be avoided if possible.

On the other hand, keeping it streamlined to a narrow set of supported language features avoids this pitfall.

I understand your position, I really didn't think about it from this perspective. But I am not convinced it's a bad idea. Users raising issue about what they consider a bug all the time :)

But its not super important for me to do it this way, maybe I could figure out how to support options by custom operation without using .Value.

If you go this route, you should also consider adding a matrix of which F# language features that are supported and which are not

README works as list of supported features. If we go this route, I think some warning that "creative" expression can lead to runtime error is in order. And maybe pointing on docs from exception.

I'll wait what @Dzoukr thinks about this and then improve docs or scratch it.

[JordanMarr]

Good point. It will probably be fine either way. 👍

[JordanMarr]

For the record, this is what I was imagining the alternative might look like:

select {
    for p in personTable do
    whereIf positionGTFilter.IsSome (p.Position > positionGTFilter.Value)
    andWhereIf positionLTFilter.IsSome (p.Position < positionLTFilter.Value)
    andWhereIf firstNameFilter.IsSome (like p.FirstName firstNameFilter.Value)
} |> conn.SelectAsync<Person>

More or less...
Just not sure if it would be better to pass in AND/OR as an argument or bake into the custom operation name.
(Note that positionGTFilter.Value wouldn't fail because the expression would only be used if the condition passed.)

Then again, this might be the most readable approach to handle conditional where statements, and it's also the closest to what you would do if writing SQL: 🤔

select {
    for p in personTable do
    where (
        (positionGTFilter.IsSome && p.Position > positionGTFilter.Value) &&
        (positionLTFilter.IsSome && p.Position < positionLTFilter.Value) &&
        (firstNameFilter.IsSome && (like p.FirstName firstNameFilter.Value))
    )
} |> conn.SelectAsync<Person>

[Dzoukr]

I see that vacation time has been productive (you, not me 😄)

Sooo, long story short, I am "team Jordan" here. The initial purpose of this library remains - keep. it. simple!

My two cents:

1. Let's have whereIf keyword with bool condition and body applied only in case bool is true. I know, I know... .Value is ugly, BUT... we should not open the Pandora box of having pattern matching in WHERE. It will work, it will be easy to maintain and future ourselves will love it in 5 months.

2. Let's talk about combining WHERE conditions. I see from @jindraivanek PR that it's also about the combination of WHERE conditions, so maybe to introduce:

andWhere (combines old WHERE with new WHERE) - may be redundant, but it's 1:1 with old WHERE (which is not combinable with other WHEREs in expression)
orWhere (the same)
andWhereIf (contra part with custom condition)
orWhereIf (the same)

What do you think? I have been away for a long time, so may be slightly slow today. 😄

[jindraivanek]

Ok, I'll make new PR with these custom operations.

I still don't like the need of .Value, but ¯\_(ツ)_/¯

@Dzoukr Should I separate it in 2 PRs:

• andWhere, orWhere
• andWhereIf, orWhereIf

or it can be in one PR?

[jindraivanek]

Closing, superseded by #94 and #95