From ebdfa9e52a8652b6aa2d3fb16f4ec4d3c3932f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gwendal=20Roue=CC=81?= Date: Sat, 11 Jan 2020 18:36:36 +0100 Subject: [PATCH] Fix failiing tests for children relations --- GRDB/QueryInterface/SQL/SQLAssociation.swift | 51 ++++++++++++++------ GRDB/QueryInterface/SQL/SQLRelation.swift | 44 ++++++++++++----- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/GRDB/QueryInterface/SQL/SQLAssociation.swift b/GRDB/QueryInterface/SQL/SQLAssociation.swift index 6419373fe9..44a6b7e920 100644 --- a/GRDB/QueryInterface/SQL/SQLAssociation.swift +++ b/GRDB/QueryInterface/SQL/SQLAssociation.swift @@ -55,7 +55,7 @@ public /* TODO: internal */ struct SQLAssociation { // All steps, from pivot to destination. Never empty. private(set) var steps: [SQLAssociationStep] var keyPath: [String] { - return steps.map { $0.key.name(for: $0.cardinality) } + return steps.map { $0.keyName } } var destination: SQLAssociationStep { @@ -189,25 +189,27 @@ public /* TODO: internal */ struct SQLAssociation { // through: Origin.hasMany(Pivot.self), // via: Pivot.belongsTo(Destination.self)) // Origin(id: 1).request(for: association) - let reversedSteps = zip(steps, steps.dropFirst()) - .map { (step, nextStep) -> SQLAssociationStep in - // Intermediate steps are not included in the selection, and - // don't have any child. - let relation = step.relation.select([]).deletingChildren() + var filteredSteps = steps + filteredSteps[0] = pivot.mapRelation { _ in filteredPivotRelation } + let reversedSteps = zip(filteredSteps, filteredSteps.dropFirst()) + .map({ (step, nextStep) -> SQLAssociationStep in + // Intermediate steps are not selected, and including(all:) + // children are useless: + let relation = step.relation + .selectOnly([]) + .filteringChildren { $0.kind.cardinality == .toOne } + + // Don't interfere with user-defined keys that could be added later + let key = step.key.mapName { "grdb_\($0)" } + return SQLAssociationStep( - key: step.key, + key: key, condition: nextStep.condition.reversed, relation: relation, - cardinality: step.cardinality) - } + cardinality: .toOne) + }) .reversed() - - var reversedAssociation = SQLAssociation(steps: Array(reversedSteps)) - // Replace pivot with the filtered one (not included in the selection, - // without children). - reversedAssociation = reversedAssociation.mapDestinationRelation { _ in - filteredPivotRelation.select([]).deletingChildren() - } + let reversedAssociation = SQLAssociation(steps: Array(reversedSteps)) return destination.relation.appendingChild(for: reversedAssociation, kind: .oneRequired) } } @@ -218,6 +220,10 @@ struct SQLAssociationStep { var relation: SQLRelation var cardinality: SQLAssociationCardinality + var keyName: String { + return key.name(for: cardinality) + } + func mapRelation(_ transform: (SQLRelation) -> SQLRelation) -> SQLAssociationStep { return SQLAssociationStep( key: key, @@ -350,4 +356,17 @@ enum SQLAssociationKey { return name } } + + func mapName(_ transform: (String) throws -> String) rethrows -> SQLAssociationKey { + switch self { + case .inflected(let name): + return try .inflected(transform(name)) + case .fixedSingular(let name): + return try .fixedSingular(transform(name)) + case .fixedPlural(let name): + return try .fixedPlural(transform(name)) + case .fixed(let name): + return try .fixed(transform(name)) + } + } } diff --git a/GRDB/QueryInterface/SQL/SQLRelation.swift b/GRDB/QueryInterface/SQL/SQLRelation.swift index ec7f167eb6..d331e50fbd 100644 --- a/GRDB/QueryInterface/SQL/SQLRelation.swift +++ b/GRDB/QueryInterface/SQL/SQLRelation.swift @@ -167,8 +167,14 @@ struct SQLRelation { case .allPrefetched: return [child.makeAssociationForKey(key)] case .oneOptional, .oneRequired, .allNotPrefetched: - return child.relation.prefetchedAssociations.map { - $0.through(child.makeAssociationForKey(key)) + return child.relation.prefetchedAssociations.map { association in + // Remove redundant pivot child + let pivotKey = association.pivot.keyName + let child = child.mapRelation { relation in + assert(relation.children[pivotKey] != nil) + return relation.removingChild(forKey: pivotKey) + } + return association.through(child.makeAssociationForKey(key)) } } } @@ -196,6 +202,14 @@ extension SQLRelation { return relation } + /// Removes all selections from chidren + func selectOnly(_ selection: [SQLSelectable]) -> SQLRelation { + var relation = self + relation.selection = selection + relation.children = relation.children.mapValues { $0.mapRelation { $0.selectOnly([]) } } + return relation + } + func annotated(with selection: [SQLSelectable]) -> SQLRelation { var relation = self relation.selection.append(contentsOf: selection) @@ -239,12 +253,6 @@ extension SQLRelation { } extension SQLRelation { - func deletingChildren() -> SQLRelation { - var relation = self - relation.children = [:] - return relation - } - /// Returns a relation extended with an association. /// /// This method provides support for public joining methods such @@ -359,6 +367,18 @@ extension SQLRelation { } return relation } + + func removingChild(forKey key: String) -> SQLRelation { + var relation = self + relation.children.removeValue(forKey: key) + return relation + } + + func filteringChildren(_ included: (Child) throws -> Bool) rethrows -> SQLRelation { + var relation = self + try relation.children = relation.children.filter { try included($1) } + return relation + } } extension SQLRelation: _JoinableRequest { @@ -616,16 +636,16 @@ struct SQLAssociationCondition: Equatable { // Join on a multiple columns. // ((table.a = 1) AND (table.b = 2)) OR ((table.a = 3) AND (table.b = 4)) ... return leftRows - .map { leftRow in + .map({ leftRow in // (table.a = 1) AND (table.b = 2) columnMappings - .map { columns -> SQLExpression in + .map({ columns -> SQLExpression in let rightColumn = QualifiedColumn(columns.right, alias: rightAlias) let leftValue = leftRow[columns.left] as DatabaseValue return rightColumn == leftValue - } + }) .joined(operator: .and) - } + }) .joined(operator: .or) } }