-
Notifications
You must be signed in to change notification settings - Fork 146
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
[WIP] Add SqlQueryBuilder wrapper #457
base: master
Are you sure you want to change the base?
Conversation
I know I'm minority in this case but I think LINQ-query wrapper is ok, and we should implement the whole LINQ support rather than limiting ourselves to some custom developer interface which is not familiar to anyone. Most of the developers can do either SQL or LINQ. Actually the API for developers may be better as SQL like https://github.com/rspeele/Rezoom.SQL has done it. People can also use direct LINQ methods with SQLProvider by opening And I think it's a good thing that we have multiple choices for a data access inside FSharp: |
That being said if this is just a facade with good integration points, it could be nice addition. One thing to consider in general is how complex nested queries we want to support. The current query didn't let me do countAsync etc. extensions. So they are now at Seq.headAsync etc, not an optimal place. |
Yes, it is as pure a wrapper around the LINQ query builder as one can write (in fact, I need to add a MIT attribution notice since I literally started by regex-replacing over In fact, if the compatibility with LINQ ever gets to 100%, it would be possible then to simply delete all of this file and replace it with And, assuming it's doable in the first place, I can see the case against adding any SQL-specific operators such as |
fsharp/fslang-suggestions#495 |
I think it's really bad UX that the library lets you use unsupported
query
operators without any compile-time warning, and you have to look in the documentation to find out which ones are missing. People keep being surprised by that.Although it would obviously be ideal to implement
groupJoin
and the other missing ones, I figured that there could be a simpler solution first: provide asqlQuery
computation expression where the unsupported operators are simply not available.The
sql
builder in this PR is a dumb calque of the originalquery
CE. The queries are still regularIQueryables
so you can freely mix and matchsql
andquery
without issue (and it doesn't break existing code). As you can see, the tests that use unsupported operators become compile-time failures.Other safety improvements that could potentially be achieved through this CE:
!!
operator with a more intuitivenaturalJoin
operator, like this:but I'm having trouble making it work - it doesn't seem like
CustomOperationAttribute
lets you define an operator that is "likefor
". Plus I'm not sure if the!!
operator would even be correctly recognized by the code inSqlRuntime.Linq.fs
this wayjoin
is only used on supported targets, by wrappingQuerySource
in a SQL-specific wrapper class?Thoughts?