From c691625b15c22efee7dcf5d05d9a5bdae9920d6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gwendal=20Roue=CC=81?= <gr@pierlis.com>
Date: Sat, 21 Apr 2018 16:53:44 +0200
Subject: [PATCH 1/4] Dedicated implementations for AND and OR sql operators.
 Introduce joined(operator:)

---
 .../QueryInterfaceRequest.swift               |   7 +-
 .../SQLExpression+QueryInterface.swift        | 105 +++++++++++-------
 .../QueryInterface/Support/SQLOperators.swift |  48 +++++---
 3 files changed, 98 insertions(+), 62 deletions(-)

diff --git a/GRDB/QueryInterface/QueryInterfaceRequest.swift b/GRDB/QueryInterface/QueryInterfaceRequest.swift
index f04147f6cc..833a2b7f5c 100644
--- a/GRDB/QueryInterface/QueryInterfaceRequest.swift
+++ b/GRDB/QueryInterface/QueryInterfaceRequest.swift
@@ -259,9 +259,6 @@ extension QueryInterfaceRequest where T: TableRecord {
         
         return QueryInterfaceRequest(query: query.mapWhereExpression { (db, expression) in
             let keyPredicates: [SQLExpression] = try keys.map { key in
-                // Prevent filter(db, keys: [[:]])
-                GRDBPrecondition(!key.isEmpty, "Invalid empty key dictionary")
-                
                 // Prevent filter(keys: [["foo": 1, "bar": 2]]) where
                 // ("foo", "bar") is not a unique key (primary key or columns of a
                 // unique index)
@@ -274,10 +271,10 @@ extension QueryInterfaceRequest where T: TableRecord {
                     // Sort key columns in the same order as the unique index
                     .sorted { (kv1, kv2) in lowercaseOrderedColumns.index(of: kv1.0.lowercased())! < lowercaseOrderedColumns.index(of: kv2.0.lowercased())! }
                     .map { (column, value) in Column(column) == value }
-                return SQLBinaryOperator.and.join(columnPredicates)! // not nil because columnPredicates is not empty
+                return columnPredicates.joined(operator: .and)
             }
             
-            let keysPredicate = SQLBinaryOperator.or.join(keyPredicates)! // not nil because keyPredicates is not empty
+            let keysPredicate = keyPredicates.joined(operator: .or)
             
             if let expression = expression {
                 return expression && keysPredicate
diff --git a/GRDB/QueryInterface/SQLExpression+QueryInterface.swift b/GRDB/QueryInterface/SQLExpression+QueryInterface.swift
index ee48475f61..35d993cc3a 100644
--- a/GRDB/QueryInterface/SQLExpression+QueryInterface.swift
+++ b/GRDB/QueryInterface/SQLExpression+QueryInterface.swift
@@ -227,20 +227,6 @@ public struct SQLBinaryOperator : Hashable {
     public static func == (lhs: SQLBinaryOperator, rhs: SQLBinaryOperator) -> Bool {
         return lhs.sql == rhs.sql
     }
-    
-    // TODO: make it an extension of Sequence (like joined(separator:)) when Swift can better handle existentials
-    // TODO: make it public eventually
-    /// Return nil if expressions is empty.
-    func join(_ expressions: [SQLExpression]) -> SQLExpression? {
-        switch expressions.count {
-        case 0:
-            return nil
-        case 1:
-            return expressions[0]
-        default:
-            return SQLExpressionBinaryOperatorChain(op: self, expressions: expressions)
-        }
-    }
 }
 
 /// [**Experimental**](http://github.com/groue/GRDB.swift#what-are-experimental-features)
@@ -325,23 +311,6 @@ public struct SQLExpressionBinary : SQLExpression {
                 return nil
             }
             
-        case .and:
-            let lids = lhs.matchedRowIds(rowIdName: rowIdName)
-            let rids = rhs.matchedRowIds(rowIdName: rowIdName)
-            switch (lids, rids) {
-            case (nil, nil): return nil
-            case let (ids?, nil), let (nil, ids?): return ids
-            case let (lids?, rids?): return lids.intersection(rids)
-            }
-            
-        case .or:
-            let lids = lhs.matchedRowIds(rowIdName: rowIdName)
-            let rids = rhs.matchedRowIds(rowIdName: rowIdName)
-            switch (lids, rids) {
-            case let (lids?, rids?): return lids.union(rids)
-            default: return nil
-            }
-            
         default:
             return nil
         }
@@ -354,29 +323,79 @@ public struct SQLExpressionBinary : SQLExpression {
     }
 }
 
-// MARK: - SQLExpressionBinaryOperatorChain
+// MARK: - SQLExpressionAnd
 
-struct SQLExpressionBinaryOperatorChain : SQLExpression {
-    let op: SQLBinaryOperator
+struct SQLExpressionAnd : SQLExpression {
     let expressions: [SQLExpression]
     
-    // Exposed to SQLBinaryOperator.join
-    fileprivate init(op: SQLBinaryOperator, expressions: [SQLExpression]) {
-        assert(expressions.count >= 2)
-        self.op = op
+    init(_ expressions: [SQLExpression]) {
         self.expressions = expressions
     }
     
     func expressionSQL(_ arguments: inout StatementArguments?) -> String {
+        guard let expression = expressions.first else {
+            // Ruby [].all? # => true
+            return true.sqlExpression.expressionSQL(&arguments)
+        }
+        if expressions.count == 1 {
+            return expression.expressionSQL(&arguments)
+        }
         let expressionSQLs = expressions.map { $0.expressionSQL(&arguments) }
-        let joiner = " \(op.sql) "
-        return "(" + expressionSQLs.joined(separator: joiner) + ")"
+        return "(" + expressionSQLs.joined(separator: " AND ") + ")"
     }
     
     func qualifiedExpression(with qualifier: SQLTableQualifier) -> SQLExpression {
-        return SQLExpressionBinaryOperatorChain(
-            op: op,
-            expressions: expressions.map { $0.qualifiedExpression(with: qualifier) })
+        return SQLExpressionAnd(expressions.map { $0.qualifiedExpression(with: qualifier) })
+    }
+    
+    func matchedRowIds(rowIdName: String?) -> Set<Int64>? {
+        let matchedRowIds = expressions.compactMap {
+            $0.matchedRowIds(rowIdName: rowIdName)
+        }
+        guard let first = matchedRowIds.first else {
+            return nil
+        }
+        return matchedRowIds.suffix(from: 1).reduce(into: first) { $0.formIntersection($1) }
+    }
+}
+
+// MARK: - SQLExpressionOr
+
+struct SQLExpressionOr : SQLExpression {
+    let expressions: [SQLExpression]
+    
+    init(_ expressions: [SQLExpression]) {
+        self.expressions = expressions
+    }
+    
+    func expressionSQL(_ arguments: inout StatementArguments?) -> String {
+        guard let expression = expressions.first else {
+            // Ruby [].any? # => false
+            return false.sqlExpression.expressionSQL(&arguments)
+        }
+        if expressions.count == 1 {
+            return expression.expressionSQL(&arguments)
+        }
+        let expressionSQLs = expressions.map { $0.expressionSQL(&arguments) }
+        return "(" + expressionSQLs.joined(separator: " OR ") + ")"
+    }
+    
+    func qualifiedExpression(with qualifier: SQLTableQualifier) -> SQLExpression {
+        return SQLExpressionOr(expressions.map { $0.qualifiedExpression(with: qualifier) })
+    }
+    
+    func matchedRowIds(rowIdName: String?) -> Set<Int64>? {
+        if expressions.isEmpty {
+            return []
+        }
+        var result: Set<Int64> = []
+        for expr in expressions {
+            guard let matchedRowIds = expr.matchedRowIds(rowIdName: rowIdName) else {
+                return nil
+            }
+            result.formUnion(matchedRowIds)
+        }
+        return result
     }
 }
 
diff --git a/GRDB/QueryInterface/Support/SQLOperators.swift b/GRDB/QueryInterface/Support/SQLOperators.swift
index 4dde080e24..b7734489d9 100644
--- a/GRDB/QueryInterface/Support/SQLOperators.swift
+++ b/GRDB/QueryInterface/Support/SQLOperators.swift
@@ -697,20 +697,12 @@ public func - (lhs: SQLSpecificExpressible, rhs: SQLSpecificExpressible) -> SQLE
 
 // MARK: - Logical Operators (AND, OR, NOT)
 
-extension SQLBinaryOperator {
-    /// The `AND` binary operator
-    static let and = SQLBinaryOperator("AND")
-    
-    /// The `OR` binary operator
-    static let or = SQLBinaryOperator("OR")
-}
-
 /// A logical SQL expression with the `AND` SQL operator.
 ///
 ///     // favorite AND 0
 ///     Column("favorite") && false
 public func && (lhs: SQLSpecificExpressible, rhs: SQLExpressible) -> SQLExpression {
-    return SQLExpressionBinary(.and, lhs.sqlExpression, rhs.sqlExpression)
+    return SQLExpressionAnd([lhs.sqlExpression, rhs.sqlExpression])
 }
 
 /// A logical SQL expression with the `AND` SQL operator.
@@ -718,7 +710,7 @@ public func && (lhs: SQLSpecificExpressible, rhs: SQLExpressible) -> SQLExpressi
 ///     // 0 AND favorite
 ///     false && Column("favorite")
 public func && (lhs: SQLExpressible, rhs: SQLSpecificExpressible) -> SQLExpression {
-    return SQLExpressionBinary(.and, lhs.sqlExpression, rhs.sqlExpression)
+    return SQLExpressionAnd([lhs.sqlExpression, rhs.sqlExpression])
 }
 
 /// A logical SQL expression with the `AND` SQL operator.
@@ -726,7 +718,7 @@ public func && (lhs: SQLExpressible, rhs: SQLSpecificExpressible) -> SQLExpressi
 ///     // email IS NOT NULL AND favorite
 ///     Column("email") != nil && Column("favorite")
 public func && (lhs: SQLSpecificExpressible, rhs: SQLSpecificExpressible) -> SQLExpression {
-    return SQLExpressionBinary(.and, lhs.sqlExpression, rhs.sqlExpression)
+    return SQLExpressionAnd([lhs.sqlExpression, rhs.sqlExpression])
 }
 
 /// A logical SQL expression with the `OR` SQL operator.
@@ -734,7 +726,7 @@ public func && (lhs: SQLSpecificExpressible, rhs: SQLSpecificExpressible) -> SQL
 ///     // favorite OR 1
 ///     Column("favorite") || true
 public func || (lhs: SQLSpecificExpressible, rhs: SQLExpressible) -> SQLExpression {
-    return SQLExpressionBinary(.or, lhs.sqlExpression, rhs.sqlExpression)
+    return SQLExpressionOr([lhs.sqlExpression, rhs.sqlExpression])
 }
 
 /// A logical SQL expression with the `OR` SQL operator.
@@ -742,7 +734,7 @@ public func || (lhs: SQLSpecificExpressible, rhs: SQLExpressible) -> SQLExpressi
 ///     // 0 OR favorite
 ///     true || Column("favorite")
 public func || (lhs: SQLExpressible, rhs: SQLSpecificExpressible) -> SQLExpression {
-    return SQLExpressionBinary(.or, lhs.sqlExpression, rhs.sqlExpression)
+    return SQLExpressionOr([lhs.sqlExpression, rhs.sqlExpression])
 }
 
 /// A logical SQL expression with the `OR` SQL operator.
@@ -750,7 +742,7 @@ public func || (lhs: SQLExpressible, rhs: SQLSpecificExpressible) -> SQLExpressi
 ///     // email IS NULL OR hidden
 ///     Column("email") == nil || Column("hidden")
 public func || (lhs: SQLSpecificExpressible, rhs: SQLSpecificExpressible) -> SQLExpression {
-    return SQLExpressionBinary(.or, lhs.sqlExpression, rhs.sqlExpression)
+    return SQLExpressionOr([lhs.sqlExpression, rhs.sqlExpression])
 }
 
 /// A negated logical SQL expression with the `NOT` SQL operator.
@@ -766,6 +758,34 @@ public prefix func ! (value: SQLSpecificExpressible) -> SQLExpression {
     return value.sqlExpression.negated
 }
 
+public enum SQLLogicalBinaryOperator {
+    case and, or
+}
+
+extension Sequence where Element: SQLExpressible {
+    /// Return an expression by joining all elements with an SQL logical
+    /// operator
+    public func joined(operator: SQLLogicalBinaryOperator) -> SQLExpression {
+        let expressions: [SQLExpression] = map { $0.sqlExpression }
+        switch `operator` {
+        case .and: return SQLExpressionAnd(expressions)
+        case .or: return SQLExpressionOr(expressions)
+        }
+    }
+}
+
+extension Sequence where Element == SQLExpression {
+    /// Return an expression by joining all elements with an SQL logical
+    /// operator
+    public func joined(operator: SQLLogicalBinaryOperator) -> SQLExpression {
+        let expressions = Array(self)
+        switch `operator` {
+        case .and: return SQLExpressionAnd(expressions)
+        case .or: return SQLExpressionOr(expressions)
+        }
+    }
+}
+
 
 // MARK: - Like Operator
 

From 729f6bdc6ced7a5b76da855456510ea19bf00a6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gwendal=20Roue=CC=81?= <gr@pierlis.com>
Date: Sat, 21 Apr 2018 17:05:03 +0200
Subject: [PATCH 2/4] Tests for joined(operator:)

---
 .../QueryInterfaceExpressionsTests.swift      | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Tests/GRDBTests/QueryInterfaceExpressionsTests.swift b/Tests/GRDBTests/QueryInterfaceExpressionsTests.swift
index 988918f8d1..b06954417f 100644
--- a/Tests/GRDBTests/QueryInterfaceExpressionsTests.swift
+++ b/Tests/GRDBTests/QueryInterfaceExpressionsTests.swift
@@ -727,6 +727,40 @@ class QueryInterfaceExpressionsTests: GRDBTestCase {
             "SELECT * FROM \"readers\" WHERE (NOT (\"age\" > 18) AND NOT (\"name\" > 'foo'))")
     }
     
+    func testJoinedOperatorAnd() throws {
+        let dbQueue = try makeDatabaseQueue()
+        
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([].joined(operator: .and))),
+            "SELECT * FROM \"readers\" WHERE 1")
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([Col.id == 1].joined(operator: .and))),
+            "SELECT * FROM \"readers\" WHERE (\"id\" = 1)")
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([Col.id == 1, Col.name != nil].joined(operator: .and))),
+            "SELECT * FROM \"readers\" WHERE ((\"id\" = 1) AND (\"name\" IS NOT NULL))")
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([Col.id == 1, Col.name != nil, Col.age == nil].joined(operator: .and))),
+            "SELECT * FROM \"readers\" WHERE ((\"id\" = 1) AND (\"name\" IS NOT NULL) AND (\"age\" IS NULL))")
+    }
+    
+    func testJoinedOperatorOr() throws {
+        let dbQueue = try makeDatabaseQueue()
+        
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([].joined(operator: .or))),
+            "SELECT * FROM \"readers\" WHERE 0")
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([Col.id == 1].joined(operator: .or))),
+            "SELECT * FROM \"readers\" WHERE (\"id\" = 1)")
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([Col.id == 1, Col.name != nil].joined(operator: .or))),
+            "SELECT * FROM \"readers\" WHERE ((\"id\" = 1) OR (\"name\" IS NOT NULL))")
+        XCTAssertEqual(
+            sql(dbQueue, tableRequest.filter([Col.id == 1, Col.name != nil, Col.age == nil].joined(operator: .or))),
+            "SELECT * FROM \"readers\" WHERE ((\"id\" = 1) OR (\"name\" IS NOT NULL) OR (\"age\" IS NULL))")
+    }
+
     
     // MARK: - String functions
     

From d63565f191e6c904ff7f7f6a0847e928b7917427 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gwendal=20Roue=CC=81?= <gr@pierlis.com>
Date: Sat, 21 Apr 2018 17:17:46 +0200
Subject: [PATCH 3/4] Documentation for joined(operator:)

---
 .../QueryInterface/Support/SQLOperators.swift | 37 ++++++++++++-------
 README.md                                     | 21 +++++++++++
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/GRDB/QueryInterface/Support/SQLOperators.swift b/GRDB/QueryInterface/Support/SQLOperators.swift
index b7734489d9..fe69256956 100644
--- a/GRDB/QueryInterface/Support/SQLOperators.swift
+++ b/GRDB/QueryInterface/Support/SQLOperators.swift
@@ -762,21 +762,30 @@ public enum SQLLogicalBinaryOperator {
     case and, or
 }
 
-extension Sequence where Element: SQLExpressible {
-    /// Return an expression by joining all elements with an SQL logical
-    /// operator
-    public func joined(operator: SQLLogicalBinaryOperator) -> SQLExpression {
-        let expressions: [SQLExpression] = map { $0.sqlExpression }
-        switch `operator` {
-        case .and: return SQLExpressionAnd(expressions)
-        case .or: return SQLExpressionOr(expressions)
-        }
-    }
-}
-
 extension Sequence where Element == SQLExpression {
-    /// Return an expression by joining all elements with an SQL logical
-    /// operator
+    /// Returns an expression by joining all elements with an SQL
+    /// logical operator.
+    ///
+    /// For example:
+    ///
+    ///     // SELECT * FROM players
+    ///     // WHERE (registered
+    ///     //        AND (score >= 1000)
+    ///     //        AND (name IS NOT NULL))
+    ///     let conditions = [
+    ///         Column("registered"),
+    ///         Column("score") >=< 1000,
+    ///         Column("name") != nil]
+    ///     Player.filter(conditions.joined(operator: .and))
+    ///
+    /// When the sequence is empty, `joined(operator: .and)` returns true,
+    /// and `joined(operator: .or)` returns false:
+    ///
+    ///     // SELECT * FROM players WHERE 1
+    ///     Player.filter([].joined(operator: .and))
+    ///
+    ///     // SELECT * FROM players WHERE 0
+    ///     Player.filter([].joined(operator: .or))
     public func joined(operator: SQLLogicalBinaryOperator) -> SQLExpression {
         let expressions = Array(self)
         switch `operator` {
diff --git a/README.md b/README.md
index db383139df..4dc8ba7e5c 100644
--- a/README.md
+++ b/README.md
@@ -3423,6 +3423,27 @@ Feed [requests](#requests) with SQL expressions built from your Swift code:
     // SELECT * FROM players WHERE ((NOT verified) OR (score < 1000))
     Player.filter(!verifiedColumn || scoreColumn < 1000)
     ```
+    
+    When you want to join a sequence of expressions with `AND` or `OR` operators, use `joined(operator:)`:
+    
+    ```swift
+    // SELECT * FROM players WHERE (verified AND (score >= 1000) AND (name IS NOT NULL))
+    let conditions = [
+        verifiedColumn,
+        scoreColumn >=< 1000,
+        nameColumn != nil]
+    Player.filter(conditions.joined(operator: .and))
+    ```
+    
+    When the sequence is empty, `joined(operator: .and)` returns true, and `joined(operator: .or)` returns false:
+    
+    ```swift
+    // SELECT * FROM players WHERE 1
+    Player.filter([].joined(operator: .and))
+    
+    // SELECT * FROM players WHERE 0
+    Player.filter([].joined(operator: .or))
+    ```
 
 - `BETWEEN`, `IN`, `NOT IN`
     

From f4ab65e399458025abd480581409c90d5553e942 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gwendal=20Roue=CC=81?= <gr@pierlis.com>
Date: Sat, 21 Apr 2018 17:21:16 +0200
Subject: [PATCH 4/4] Fix typo

---
 GRDB/QueryInterface/Support/SQLOperators.swift | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/GRDB/QueryInterface/Support/SQLOperators.swift b/GRDB/QueryInterface/Support/SQLOperators.swift
index fe69256956..f0e8a78dbf 100644
--- a/GRDB/QueryInterface/Support/SQLOperators.swift
+++ b/GRDB/QueryInterface/Support/SQLOperators.swift
@@ -774,7 +774,7 @@ extension Sequence where Element == SQLExpression {
     ///     //        AND (name IS NOT NULL))
     ///     let conditions = [
     ///         Column("registered"),
-    ///         Column("score") >=< 1000,
+    ///         Column("score") >= 1000,
     ///         Column("name") != nil]
     ///     Player.filter(conditions.joined(operator: .and))
     ///