Merge 3.1 to master for 3.2.0-M2 #1661
Conversation
On databases which support `DISTINCT ON` (like PostgreSQL) we skip the `rewriteDistinct` phase and the queries already collapse nicely to a single comprehension. In many cases the `DISTINCT ON` can still be rewritten to a `DISTINCT` later in the code generator (if the columns match up). On databases that do not support `DISTINCT ON` (like MySQL) we have to eliminate these operations early on (in `rewriteDistinct`) and replace them by either `DISTINCT` (where possible) or `GROUP BY`. The catch is that we have to inject an artificial subquery boundary on top of a `DISTINCT` to prevent mappings from being applied across the `DISTINCT` (which could change the set of columns that determine distinctness). This subquery boundary then prevents the `Take` operation from being merged into the existing comprehension in `mergeComprehensions`. The solution is to push the always distinctness-preserving operations `Take` and `Drop` down under `Subquery.AboveDistinct` in `reorderOperations`. The test case (`q7` in `AggregateTest.testDistinct`) also triggers a bug when running on the special H2Rownum test profile: When `resolveZipJoins` uses `rownumStyle=true` you can end up with a `Subquery.BelowRownum` boundary between a `Distinct` and its enclosing `Bind`, in which case `rewriteDistinct` doesn't perform the rewriting. The solution is to ensure that there is always a `Bind` below the boundary in `resolveZipJoins` and create an identity `Bind` where necessary to preserve this invariant (which should hold in all phases between `forceOuterBinds` and `mergeToComprehensions`).
Merge `LIMIT` on top of `DISTINCT`
- NotNull and Null no longer available - DBType is SqlType
Fixed documentation on ColumnOptions
Conflicts: slick-testkit/src/main/scala/com/typesafe/slick/testkit/tests/AggregateTest.scala slick/src/sphinx/schemas.rst
|
WTF is going on now? AppVeyor can't find sbt again (after I fixed it yesterday), and now Travis is also broken for no obvious reason. |
|
Several failures around zip joins on SQL Server. Apparently 5cfbd2e interacts badly with some other change in master. |
|
Or more likely: It never worked on SQL Server. 3.1 still needs Slick Extensions for that, so it wasn't routinely tested in CI. |
- Use the new MySQL 5.6 package on Travis. - Do not cache `c:\sbt` at all on AppVeyor. Purging the cache when switching between branches (which install different sbt versions into different locations) doesn’t seem to work.
szeiger
force-pushed
the
tmp/merge-3.1-to-master-for-3.2.0-M2
branch
from
6ecc00b
to
667ffd5
Compare
|
@smootoo Any idea what's up with the Oracle connection? I've occasionally seen this problem before but a restart always fixed it. Now the pool startup consistently hangs even though the docker image is up: https://travis-ci.org/slick/slick/builds/180734597 |
|
@szeiger I'll take a look at this over the weekend |
|
@smootoo Thanks. Looks like it's working again now (without any changes to the branch) |
|
For the appveyor fails, that looks like a real break with the SQLServer profiles. The sorting on the JoinTest.testZip is broken. It’s probably not too hard to fix, but in the 6 months since I last did any scala dev work, all my SQLServer VMs seemed to have stopped working :-( I’ll need to configure a new test environment, which will take a few days, but I’m happy to take a look at it if you would like? Regarding the pool hanging on startup of Oracle. In my testing, I’ve not been able to reproduce it, but can see it hanging on the link you provided. It’s trying to acquire a database connection through Hikari. Even if the docker image had stalled, I would have expected a timeout, but looking at the configuration options I put into the test config, they are very conservative. A 10 min connection timeout is the same as the travis stalled build timeout threshold. I'll try some less pessimistic Hikari connection timeouts and maybe the pool can handle those connection timeouts and let the builds complete. |
|
The SQL Server failures are very likely due to 5cfbd2e. I'm working my way down through the rabbit hole to fix them. After making everything work again with updates on my Windows VM, OSX, Travis, changes in Slick 3.1 and Slick master (the last of which was #1582 (comment)), I can finally reproduce them locally. |
This was introduced as part of slick#1582 but requires at least PostgreSQL 9.2.
The test fails since slick#1649 was merged but we didn’t notice because it went into the 3.1 branch which does not yet run on what were then the Slick Extensions drivers. This failure demonstrates that even with the new backend in 3.1 it is still hard to predict all possible results when it comes to comprehension fusion. The cause of the regression is not actually the new reordering from slick#1649 but the fix for `h2rownum` that was necessary as a result of it. The extra identity mapping prevented `reorderOperations` from pushing a `Subquery.BelowRownum` into a `SortBy` (because there is now an identity mapping in between the two) and then eliminating it entirely when it hits the `TableNode` below. This led to an unnecessary subquery, which would have gone unnoticed if Slick was better at propagating sort order through subqueries. There is an existing rewrite rule in `reorderOperations` to push aliasing mappings under subqueries that are not of type `AboveDistinct`. I added another exception for `BelowRowNumber` and a rule that does the opposite, i.e. push `Subquery.BelowRowNumber` into aliasing mappings just like we already push it into `SortBy` and `Filter`. Since the goal of these rules is to push it down so far that it eventually hits a `TableNode` (where it can be eliminated), this seems like the right direction to take.
|
At last, the AppVeyor build passed. Once the others are green, too, I'll merge and cut the release, then cherry-pick the last commit back to 3.1. |
No description provided.