MS SQL - take 3

[aldum]

This PR builds on the work in #29 and #60 by @kiendang and @jtjeferreira.

What it does:

• rebase previous works on top of current main
• fix table name escape
• fix DataTypesTests and OptionalsTests with the point of contention being the lack of a dedicated boolean column type
• add more tests (inserting both values of boolean, updating BIT columns) and document any result deviations

How this is achieved:

T-SQL (the dialect used by MS SQL Server) does have Booleans as part of filter expressions, but not as column data, with BITs being used as a substitute. Hence, these uses which are both Boolean on the Scala side, need to be distinguished. To this end, a marker was introduced in Context, explicitly designating InsertValues, InsertSelect, InsertColumns and Update expression as using them as values. This was done globally, because overloading would lead to a lot more boilerplate, while simply disregarding the flags value leave the existing dialects intact.

Known omissions:

• ExprBooleanOps for BIT values: small edge case, it's possible and there's syntax for it (bitwise operators), but it needs another branching of the above kind, to not break the normal OR/AND/NOT syntax. Bitwise operators are also already tested as part of NumericOps.
• RETURNING: this is not a thing in T-SQL, however it does have OUTPUT, which could be used to achieve similar results
• ON CONFLICT: similarly, not present in T-SQL, there is MERGE in the standard, but the opinions are mixed on whether it achieves the adequate level of isolation as Postgres's ON CONFLICT does.
• LATERAL JOIN: again, very Postgres, maybe CROSS APPLY could be used to achieve similar effect.
• bare VALUES: not supported. However, it is not it's own Dialect trait yet, which would indicate this more cleanly,

[kiendang]

Sorry for the late review. Thanks for finishing up the PR.

Back when I was working on this the biggest problem was handling Boolean. I also thought of having a marker to differentiate Boolean in different types of clauses, e.g. WHERE vs SELECT, like what you've done here. My concern at that time was subqueries, e.g. a WHERE clause with a SELECT subquery that returns Boolean. However I couldn't think of a specific query/use case where such a clause would be useful or cannot be rewritten in a better way.

• .filter(_ => Expr(true)) would fail with MSSQL, but the clause itself is useless, and would even be optimized away if rewritten as .filter(_ => true).
• Another query that I think would also probably fail is Buyer.select.filter(c => Buyer.select.map(c => c.id `=` 3).sortBy(c => c).desc.take(1).toExpr) (I haven't actually run this yet), but it's also not practical.

This is the kind of select inside where subquery that is actually practical, which is already handled correctly.

scalasql/scalasql/test/src/query/SubQueryTests.scala

Line 167 in 34678e2

test("subqueryInFilter") - checker(

Thus I agree with the handling of Boolean done in this PR.

[lihaoyi]

Thanks @kiendang

@aldum I think this looks good overall. Could you flesh out the PR description:

• What were the unsolved problems in the previous attempts that you built upon
• What you did to solve those problems
• Any interesting design decisions that are worth highlighting
• What are the known limitations of this PR

Also add comments in the relevant places in the code

• For the suites that are commented out in ConcreteTestSuites.scala, please add a one-line comment before each one explaining why it is commented out
• For the test cases you added alternate result values to, please add a comment explaining why the query produces a different result in MS Sql Server

[aldum]

Thanks for looking at it, @jtjeferreira, @kiendang, @lihaoyi !

I will move this comment into the description and reword a bit.

The requested comments were added in the commits above.

[lihaoyi]

@aldum I think this looks great, thanks for your work.

For the bounty, @kiendang has already received 500USD for his work earlier, and I'll split the remaining 1000USD bounty between @aldum and @jtjeferreira at 500USD each since all of you contributed significantly to the final PR

@aldum @jtjeferreira do email me your international bank transfer credentials and I will close out the bounty