ensure the LEFT JOIN behaves as expected

The WHERE clause will filter out these rows because NULL values do not satisfy either the IS NULL or the greater than conditions, effectively excluding them from the result set. Turning the LEFT JOIN into an INNER JOIN for rows where NULL values would have been expected. * move includeDeleted checks from query to join, where the filter will be applied * copy filters over and add filter as necessary, on case deleted in not included * keep includeDeleted checks within query aswell, as null checks now can exists in where clause aswell. * adjust tests to reflect changes in code.
vapor · Oct 31, 2024 · fa72055 · fa72055
1 parent 614d3ec
commit fa72055
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 7 deletions.
diff --git a/Sources/FluentKit/Properties/Timestamp.swift b/Sources/FluentKit/Properties/Timestamp.swift
@@ -208,6 +208,7 @@ extension Fields {
 }
 
 extension Schema {
+
     static func excludeDeleted(from query: inout DatabaseQuery) {
         guard let timestamp = self.init().deletedTimestamp else {
             return
@@ -222,4 +223,26 @@ extension Schema {
             .value(deletedAtField, .greaterThan, timestamp.currentTimestampInput)
         ], .or))
     }
+
+
+    static func excludeDeleted(from filters: [DatabaseQuery.Filter]) -> [DatabaseQuery.Filter] {
+        guard let timestamp = self.init().deletedTimestamp else {
+            return filters
+        }
+
+        let deletedAtField = DatabaseQuery.Field.extendedPath(
+            [timestamp.key],
+            schema: self.schemaOrAlias,
+            space: self.space
+        )
+
+        var copy = filters
+        copy.append(.group([
+            .value(deletedAtField, .equal, .null),
+            .value(deletedAtField, .greaterThan, timestamp.currentTimestampInput)
+        ], .or))
+
+        let filters = copy
+        return filters
+    }
 }
diff --git a/Sources/FluentKit/Query/Builder/QueryBuilder+Join.swift b/Sources/FluentKit/Query/Builder/QueryBuilder+Join.swift
@@ -36,7 +36,16 @@ extension QueryBuilder {
     ) -> Self
         where Foreign: Schema
     {
-        self.join(Foreign.self, on: .advancedJoin(schema: Foreign.schema, space: Foreign.space, alias: Foreign.alias, method, filters: filters))
+
+        // If deleted models aren't included, add filters
+        // to exclude them for each model being queried.
+        if !self.includeDeleted {
+            let filters = foreign.excludeDeleted(from: filters)
+
+            return self.join(Foreign.self, on: .advancedJoin(schema: Foreign.schema, space: Foreign.space, alias: Foreign.alias, method, filters: filters))
+        }
+
+        return self.join(Foreign.self, on: .advancedJoin(schema: Foreign.schema, space: Foreign.space, alias: Foreign.alias, method, filters: filters))
     }
 
     /// `.join(Foreign.self, on: databaseJoin)`

diff --git a/Sources/FluentKit/Query/Builder/QueryBuilder.swift b/Sources/FluentKit/Query/Builder/QueryBuilder.swift
@@ -303,10 +303,11 @@ public final class QueryBuilder<Model>
                 self.addFields(for: model, to: &query)
             }
         }
-
+
+
         // If deleted models aren't included, add filters
         // to exclude them for each model being queried.
-        if !self.includeDeleted {
+        if  !self.includeDeleted {
             for model in self.models {
                 model.excludeDeleted(from: &query)
             }

diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Action.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Action.swift
@@ -1,5 +1,24 @@
 extension DatabaseQuery {
-    public enum Action: Sendable {
+    public enum Action: Sendable, Equatable {
+
+        public static func == (lhs: DatabaseQuery.Action, rhs: DatabaseQuery.Action) -> Bool {
+            switch (lhs, rhs) {
+            case (.create, .create),
+                (.read, .read),
+                (.update, .update),
+                (.delete, .delete):
+                return true
+            case let (.aggregate(lhs), .aggregate(rhs)):
+                guard type(of: lhs) == type(of: rhs) else { return false }
+                return String(describing: lhs) == String(describing: rhs)
+            case let (.custom(lhs), .custom(rhs)):
+                guard type(of: lhs) == type(of: rhs) else { return false }
+                return String(describing: lhs) == String(describing: rhs)
+            default:
+                return false
+            }
+        }
+
         case create
         case read
         case update

diff --git a/Tests/FluentKitTests/CompositeIDTests.swift b/Tests/FluentKitTests/CompositeIDTests.swift
@@ -120,7 +120,7 @@ final class CompositeIDTests: XCTestCase {
         XCTAssertEqual(db.sqlSerializers.count, 9)
         XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"SELECT "planets"."id" AS "planets_id", "planets"."name" AS "planets_name", "planets"."star_id" AS "planets_star_id" FROM "planets" WHERE "planets"."id" = $1 LIMIT 1"#)
         XCTAssertEqual(try db.sqlSerializers.xctAt(1).sql, #"SELECT "composite+planet+tag"."planet_id" AS "composite+planet+tag_planet_id", "composite+planet+tag"."tag_id" AS "composite+planet+tag_tag_id", "composite+planet+tag"."notation" AS "composite+planet+tag_notation", "composite+planet+tag"."createdAt" AS "composite+planet+tag_createdAt", "composite+planet+tag"."updatedAt" AS "composite+planet+tag_updatedAt", "composite+planet+tag"."deletedAt" AS "composite+planet+tag_deletedAt" FROM "composite+planet+tag" WHERE "composite+planet+tag"."planet_id" = $1 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $2)"#)
-        XCTAssertEqual(try db.sqlSerializers.xctAt(2).sql, #"SELECT "tags"."id" AS "tags_id", "tags"."name" AS "tags_name", "composite+planet+tag"."planet_id" AS "composite+planet+tag_planet_id", "composite+planet+tag"."tag_id" AS "composite+planet+tag_tag_id", "composite+planet+tag"."notation" AS "composite+planet+tag_notation", "composite+planet+tag"."createdAt" AS "composite+planet+tag_createdAt", "composite+planet+tag"."updatedAt" AS "composite+planet+tag_updatedAt", "composite+planet+tag"."deletedAt" AS "composite+planet+tag_deletedAt" FROM "tags" INNER JOIN "composite+planet+tag" ON "tags"."id" = "composite+planet+tag"."tag_id" WHERE "composite+planet+tag"."planet_id" = $1 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $2)"#)
+        XCTAssertEqual(try db.sqlSerializers.xctAt(2).sql, #"SELECT "tags"."id" AS "tags_id", "tags"."name" AS "tags_name", "composite+planet+tag"."planet_id" AS "composite+planet+tag_planet_id", "composite+planet+tag"."tag_id" AS "composite+planet+tag_tag_id", "composite+planet+tag"."notation" AS "composite+planet+tag_notation", "composite+planet+tag"."createdAt" AS "composite+planet+tag_createdAt", "composite+planet+tag"."updatedAt" AS "composite+planet+tag_updatedAt", "composite+planet+tag"."deletedAt" AS "composite+planet+tag_deletedAt" FROM "tags" INNER JOIN "composite+planet+tag" ON "tags"."id" = "composite+planet+tag"."tag_id" AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $1) WHERE "composite+planet+tag"."planet_id" = $2 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $3)"#)
         XCTAssertEqual(try db.sqlSerializers.xctAt(3).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, $3, $4, DEFAULT)"#)
         XCTAssertEqual(try db.sqlSerializers.xctAt(4).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, $3, $4, DEFAULT)"#)
         XCTAssertEqual(try db.sqlSerializers.xctAt(5).sql, #"SELECT COUNT("composite+planet+tag"."planet_id") AS "aggregate" FROM "composite+planet+tag" WHERE "composite+planet+tag"."planet_id" = $1 AND "composite+planet+tag"."tag_id" = $2 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $3)"#)

diff --git a/Tests/FluentKitTests/FluentKitTests.swift b/Tests/FluentKitTests/FluentKitTests.swift
@@ -739,7 +739,7 @@ final class FluentKitTests: XCTestCase {
         XCTAssertEqual(db.sqlSerializers.dropFirst(2).first?.sql, #"INSERT INTO "mirror_universe"."planets" ("id", "name", "star_id", "possible_star_id", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, DEFAULT, $3, $4, DEFAULT)"#)
         XCTAssertEqual(db.sqlSerializers.dropFirst(3).first?.sql, #"UPDATE "mirror_universe"."planets" SET "id" = $1, "name" = $2, "updatedAt" = $3 WHERE "mirror_universe"."planets"."id" = $4 AND ("mirror_universe"."planets"."deletedAt" IS NULL OR "mirror_universe"."planets"."deletedAt" > $5)"#)
         XCTAssertEqual(db.sqlSerializers.dropFirst(4).first?.sql, #"DELETE FROM "mirror_universe"."planets" WHERE "mirror_universe"."planets"."name" <> $1"#)
-        XCTAssertEqual(db.sqlSerializers.dropFirst(5).first?.sql, #"SELECT "stars"."id" AS "stars_id", "stars"."name" AS "stars_name", "stars"."galaxy_id" AS "stars_galaxy_id", "stars"."deleted_at" AS "stars_deleted_at" FROM "stars" INNER JOIN "mirror_universe"."planets" ON "mirror_universe"."planets"."star_id" = "stars"."id" LIMIT 1"#)
+        XCTAssertEqual(db.sqlSerializers.dropFirst(5).first?.sql, #"SELECT "stars"."id" AS "stars_id", "stars"."name" AS "stars_name", "stars"."galaxy_id" AS "stars_galaxy_id", "stars"."deleted_at" AS "stars_deleted_at" FROM "stars" INNER JOIN "mirror_universe"."planets" ON "mirror_universe"."planets"."star_id" = "stars"."id" AND ("mirror_universe"."planets"."deletedAt" IS NULL OR "mirror_universe"."planets"."deletedAt" > $1) LIMIT 1"#)
     }
 
     func testKeyPrefixingStrategies() throws {

diff --git a/Tests/FluentKitTests/QueryBuilderTests.swift b/Tests/FluentKitTests/QueryBuilderTests.swift
@@ -206,6 +206,6 @@ final class QueryBuilderTests: XCTestCase {
             .join(Star.self, on: \Star.$id == \Planet.$star.$id && \Star.$name != \Planet.$name)
             .all().wait()
         XCTAssertEqual(db.sqlSerializers.count, 1)
-        XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"SELECT "planets"."id" AS "planets_id", "planets"."name" AS "planets_name", "planets"."star_id" AS "planets_star_id", "planets"."possible_star_id" AS "planets_possible_star_id", "planets"."deleted_at" AS "planets_deleted_at", "stars"."id" AS "stars_id", "stars"."name" AS "stars_name", "stars"."galaxy_id" AS "stars_galaxy_id", "stars"."deleted_at" AS "stars_deleted_at" FROM "planets" INNER JOIN "stars" ON "stars"."id" = "planets"."star_id" AND "stars"."name" <> "planets"."name" WHERE ("planets"."deleted_at" IS NULL OR "planets"."deleted_at" > $1) AND ("stars"."deleted_at" IS NULL OR "stars"."deleted_at" > $2)"#)
+        XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"SELECT "planets"."id" AS "planets_id", "planets"."name" AS "planets_name", "planets"."star_id" AS "planets_star_id", "planets"."possible_star_id" AS "planets_possible_star_id", "planets"."deleted_at" AS "planets_deleted_at", "stars"."id" AS "stars_id", "stars"."name" AS "stars_name", "stars"."galaxy_id" AS "stars_galaxy_id", "stars"."deleted_at" AS "stars_deleted_at" FROM "planets" INNER JOIN "stars" ON "stars"."id" = "planets"."star_id" AND "stars"."name" <> "planets"."name" AND ("stars"."deleted_at" IS NULL OR "stars"."deleted_at" > $1) WHERE ("planets"."deleted_at" IS NULL OR "planets"."deleted_at" > $2) AND ("stars"."deleted_at" IS NULL OR "stars"."deleted_at" > $3)"#)
     }
 }