Add support for the WITH clause #1139
Conversation
The `WITH` clause provides the ability to do hierarchical or recursive queries of tree and graph-like data. See https://www.sqlite.org/lang_with.html. **Details** - Add `all` parameter to `QueryType.union` to allow `UNION ALL` to be used in a query. I opted to add the parameter to the start of the list so that it does not dangle at the end when the union's query is long: ```swift users.union(all: true, posts.join(users, on: users[id] == posts[userId])) // It's a little easier to read than: users.union(posts.join(users, on: users[id] == posts[userId]), all: true) ``` - Add `with` function to `QueryType`. This function adds a `WITH` clause to a query. The function may be called multiple times to add multiple clauses to a query. If multiple clauses are added to the query with conflicting `recursive` parameters, the whole `WITH` clause will be considered recursive. Like the `union` function, I put the `subquery` parameter at the end so that the `recursive` and `materializationHint` options don't dangle at the end of a long query. ```swift let users = Table("users") let users = Table("posts") let first = Table("first") let second = Table("second") first.with(first, recursive: true as: users).with(second, recursive: false, as: posts) // WITH RECURSIVE "first" AS (SELECT * from users), "second" AS (SELECT * from posts) SELECT * from "first" ```
Note sure how to get the line length of Query back down below 500 without harming readability though.
|
@marmphco thanks for the PR. the simplest solution for the linter problem would be to start splitting |
|
@jberkel Good point, I'll try that out. |
Sources/SQLite/Typed/Query.swift
Outdated
| // MARK: - Private | ||
|
|
||
| struct WithClauses { |
There was a problem hiding this comment.
these could be moved to the new file as well
There was a problem hiding this comment.
Got it, will move. Wasn't sure whether to move them since they kind of feel like part of QueryClauses.
There was a problem hiding this comment.
MaterializationHint probably as well since it's only used there
There was a problem hiding this comment.
🤦 of course, my bad!
| @@ -59,6 +59,11 @@ class QueryTests: XCTestCase { | |||
| assertSQL("SELECT DISTINCT * FROM \"users\"", users.select(distinct: *)) | |||
| } | |||
|
|
|||
| func test_union_compilesUnionClause() { | |||
There was a problem hiding this comment.
split:
test_union_compiles_Union_Clause
test_union_compiles_Union_All_Clause
There was a problem hiding this comment.
I split the tests in cfa6010. I kept the names in the form test_union_compilesUnionAllClause for consistency with the existing tests. Let me know if I should instead do test_union_compiles_Union_All_Clause.
Tests/SQLiteTests/QueryTests.swift
Outdated
| temp.with(temp, as: users)) | ||
| } | ||
|
|
||
| func test_with_recursive_compilesWithClause() { |
There was a problem hiding this comment.
compiles_with_clause
compiles_with_recursive_clause
Tests/SQLiteTests/QueryTests.swift
Outdated
| temp.with(temp, recursive: false, as: users)) | ||
| } | ||
|
|
||
| func test_with_materialization_compilesWithClause() { |
There was a problem hiding this comment.
with_materialized
with_not_materialized
| /// - table: A query representing the other table. | ||
| /// | ||
| /// - Returns: A query with the given `UNION` clause applied. | ||
| public func union(_ table: QueryType) -> Self { | ||
| public func union(all: Bool = false, _ table: QueryType) -> Self { |
There was a problem hiding this comment.
I agree that putting all in front of the parameter list is clearer here (and mimics the SQL), but as you say this will break API compatibility, so we'll have to release it as 0.14.
In stephencelis#1139 I introduced support for the `WITH` clause. My implementation contains a bug: the statement preparer doesn't produce the correct result column names for queries containing a `SELECT *` preceded by a `WITH`. For example, consider the following statement: ``` WITH temp AS ( SELECT id, email from users) SELECT * from temp ``` An error would be thrown when preparing this statement because the glob expansion procedure would try to look up the column names for the result by looking up the column names for the query `SELECT * from temp`. This does not work because `temp` is a temporary view defined in the `WITH` clause. To fix this, I modified the glob expansion procedure to include the `WITH` clause in the query used to look up the result column names.
Hello! I have been using SQLite.swift in one of my projects and so far it's been working great. Just one thing is missing that my project needs: the
WITHclause. TheWITHclause provides the ability to do hierarchical or recursive queries of tree and graph-like data. See https://www.sqlite.org/lang_with.html.Thanks in advance for reviewing 🙂
API changes
Add an
allparameter toQueryType.unionto allowUNION ALLto be used in a query.UNION ALLallows duplicate rows in the result, which may help improve performance for large recursive queries. I opted to add the parameter to the start of the list so that it does not dangle at the end when the union's query is long:Add
withfunction toQueryType. This function adds aWITHclause to a query. The function may be called multiple times to add multiple clauses to a query. If multiple clauses are added to the query with conflictingrecursiveparameters, the wholeWITHclause will be considered recursive.Like the
unionfunction, I put the query parameter at the end so that therecursiveandmaterializationHintoptions don't dangle at the end of a long query.Other
I left one linter warning unfixed: there's a file length violation in Query.swift. The file is 519 lines long, 19 over the limit. I'm not sure how to bring the line length back down below 500 without harming readability or moving a lot of stuff around (maybe you do?). Any suggestions on what to do here?