Skip to content

Commit

Permalink
fix(datastore): deduplicate SQL statement of attributeExists and eq/ne
Browse files Browse the repository at this point in the history
  • Loading branch information
5d committed Aug 27, 2024
1 parent 574b758 commit 31d507d
Show file tree
Hide file tree
Showing 7 changed files with 214 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ extension QueryOperator: Equatable {
case let (.between(oneStart, oneEnd), .between(otherStart, otherEnd)):
return PersistableHelper.isEqual(oneStart, otherStart)
&& PersistableHelper.isEqual(oneEnd, otherEnd)
case let (.attributeExists(one), .attributeExists(other)):
return one == other
default:
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,56 @@ extension GraphQLModelBasedTests {
XCTAssertNotNil(error)
}
}

/**
- Given: API with Post schema and optional field 'draft'
- When:
- create a new post with optional field 'draft' value .none
- Then:
- query Posts with filter {eq : null} shouldn't include the post
*/
func test_listModelsWithNilOptionalField_failedWithEqFilter() async throws {
let post = Post(title: UUID().uuidString, content: UUID().uuidString, createdAt: .now())
_ = try await Amplify.API.mutate(request: .create(post))
let posts = try await list(.list(
Post.self,
where: Post.keys.draft == nil && Post.keys.createdAt >= post.createdAt
))

XCTAssertFalse(posts.map(\.id).contains(post.id))
}

/**
- Given: DataStore with Post schema and optional field 'draft'
- When:
- create a new post with optional field 'draft' value .none
- Then:
- query Posts with filter {attributeExists : false} should include the post
*/
func test_listModelsWithNilOptionalField_successWithAttributeExistsFilter() async throws {
let post = Post(title: UUID().uuidString, content: UUID().uuidString, createdAt: .now())
_ = try await Amplify.API.mutate(request: .create(post))
let listPosts = try await list(
.list(
Post.self,
where: Post.keys.draft.attributeExists(false)
&& Post.keys.createdAt >= post.createdAt
)
)

XCTAssertTrue(listPosts.map(\.id).contains(post.id))
}

func list<M: Model>(_ request: GraphQLRequest<List<M>>) async throws -> [M] {
func getAllPages(_ list: List<M>) async throws -> [M] {
if list.hasNextPage() {
return list.elements + (try await getAllPages(list.getNextPage()))
} else {
return list.elements
}
}

return try await getAllPages(try await Amplify.API.query(request: request).get())
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class GraphQLListQueryTests: XCTestCase {
func test_listQuery_withAttributeExistsFilter_correctlyBuildGraphQLQueryStatement() {
let post = Post.keys
let predicate = post.id.eq("id")
&& (post.draft.attributeExists(false) || post.draft.eq("null"))
&& (post.draft.attributeExists(false) || post.draft.eq(nil))

var documentBuilder = ModelBasedGraphQLDocumentBuilder(modelSchema: Post.schema, operationType: .query)
documentBuilder.add(decorator: DirectiveNameDecorator(type: .list))
Expand Down Expand Up @@ -292,7 +292,7 @@ class GraphQLListQueryTests: XCTestCase {
},
{
"draft" : {
"eq" : "null"
"eq" : null
}
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ extension QueryOperator {
case .notContains:
return "instr(\(column), ?) = 0"
case .attributeExists(let value):
return "\(column) is \(value ? "not" : "") null"
return "\(column) is\(value ? " not" : "") null"
}
}

Expand All @@ -53,8 +53,8 @@ extension QueryOperator {
.beginsWith(let value),
.notContains(let value):
return [value.asBinding()]
case .attributeExists:
return []
case .attributeExists(let value):
return [value.asBinding()]
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,66 @@ private func translateQueryPredicate(from modelSchema: ModelSchema,
return operation.field.quoted()
}

func optimizeQueryPredicateGroup(_ predicate: QueryPredicate) -> QueryPredicate {
func rewritePredicate(_ predicate: QueryPredicate) -> QueryPredicate {
if let operation = predicate as? QueryPredicateOperation {
switch operation.operator {
case .attributeExists(let bool):
return QueryPredicateOperation(
field: operation.field,
operator: bool ? .notEqual(nil) : .equals(nil)
)
default:
return operation
}
} else if let group = predicate as? QueryPredicateGroup {
return optimizeQueryPredicateGroup(group)
}

return predicate
}

func removeDuplicatePredicate(_ predicates: [QueryPredicate]) -> [QueryPredicate] {
var result = [QueryPredicate]()
for predicate in predicates {
let hasSameExpression = result.reduce(false) {
if $0 { return $0 }
switch ($1, predicate) {
case let (lhs as QueryPredicateOperation, rhs as QueryPredicateOperation):
return lhs == rhs
case let (lhs as QueryPredicateGroup, rhs as QueryPredicateGroup):
return lhs == rhs
default:
return false
}
}

if !hasSameExpression {
result.append(predicate)
}
}
return result
}

switch predicate {
case let predicate as QueryPredicateGroup:
let optimizedPredicates = removeDuplicatePredicate(predicate.predicates.reduce([]) {
$0 + [rewritePredicate($1)]
})

if optimizedPredicates.count == 1 {
return optimizedPredicates.first!
} else {
return QueryPredicateGroup(type: predicate.type, predicates: optimizedPredicates)
}
default:
return predicate
}
}

// the very first `and` is always prepended, using -1 for if statement checking
// the very first `and` is to connect `where` clause with translated QueryPredicate
translate(predicate, predicateIndex: -1, groupType: .and)
translate(optimizeQueryPredicateGroup(predicate), predicateIndex: -1, groupType: .and)
return (sql.joined(separator: "\n"), bindings)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1350,4 +1350,101 @@ class SQLStatementTests: XCTestCase {
XCTAssertEqual(statement.stringValue, expectStatement)
XCTAssertEqual(variables[0] as? String, expectedVariable)
}


/// Given: a query predicate of attributeExists
/// When: the bind value is false
/// Then: generate the correct SQL query statement
func test_translateAttributeExistsFalseQueryPredicate() {
let post = Post.keys

let predicate = post.id.attributeExists(false)
let statement = ConditionStatement(modelSchema: Post.schema, predicate: predicate, namespace: "root")
let expectedStatement =
"""
and "root"."id" is null
"""
XCTAssertEqual(statement.stringValue, expectedStatement)
}

/// Given: a query predicate of attributeExists
/// When: the bind value is true
/// Then: generate the correct SQL query statement
func test_translateAttributeExistsTrueQueryPredicate() {
let post = Post.keys

let predicate = post.id.attributeExists(true)
let statement = ConditionStatement(modelSchema: Post.schema, predicate: predicate, namespace: "root")
let expectedStatement =
"""
and "root"."id" is not null
"""
XCTAssertEqual(statement.stringValue, expectedStatement)
}

/// Given: a combined query predicate of attributeExists and ne
/// When: attributeExists(true) && ne(nil)
/// Then: generate the correct SQL query statement
func test_translateCombinedQueryPredicateOfAttributeExistsTrueAndNeNil() {
let post = Post.keys

let predicate = post.id.attributeExists(true) && post.id.ne(nil)
let statement = ConditionStatement(modelSchema: Post.schema, predicate: predicate, namespace: "root")
let expectedStatement =
"""
and "root"."id" is not null
"""
XCTAssertEqual(statement.stringValue, expectedStatement)
}

/// Given: a combined query predicate of attributeExists and ne
/// When: attributeExists(false) && ne(nil)
/// Then: generate the correct SQL query statement
func test_translateCombinedQueryPredicateOfAttributeExistsFalseAndNeNil() {
let post = Post.keys

let predicate = post.id.attributeExists(false) && post.id.ne(nil)
let statement = ConditionStatement(modelSchema: Post.schema, predicate: predicate, namespace: "root")
let expectedStatement =
"""
and (
"root"."id" is null
and "root"."id" is not null
)
"""
XCTAssertEqual(statement.stringValue, expectedStatement)
}

/// Given: a combined query predicate of attributeExists and eq
/// When: attributeExists(false) || eq(nil)
/// Then: generate the correct SQL query statement
func test_translateCombinedQueryPredicateOfAttributeExistsFalseOrEqNil() {
let post = Post.keys

let predicate = post.id.attributeExists(false) || post.id.eq(nil)
let statement = ConditionStatement(modelSchema: Post.schema, predicate: predicate, namespace: "root")
let expectedStatement =
"""
and "root"."id" is null
"""
XCTAssertEqual(statement.stringValue, expectedStatement)
}

/// Given: a combined query predicate of attributeExists and eq
/// When: attributeExists(true) || eq(nil)
/// Then: generate the correct SQL query statement
func test_translateCombinedQueryPredicateOfAttributeExistsTrueOrEqNil() {
let post = Post.keys

let predicate = post.id.attributeExists(true) || post.id.eq(nil)
let statement = ConditionStatement(modelSchema: Post.schema, predicate: predicate, namespace: "root")
let expectedStatement =
"""
and (
"root"."id" is not null
or "root"."id" is null
)
"""
XCTAssertEqual(statement.stringValue, expectedStatement)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -650,56 +650,6 @@ class DataStoreEndToEndTests: SyncEngineIntegrationTestBase {
XCTAssertEqual(localSuccess, postCount)
}

/**
- Given: DataStore with Post schema and optional field 'draft'
- When:
- create a new post with optional field 'draft' value .none
- Then:
- query Posts with filter {eq : "null"} shouldn't include the post
*/
func test_listModelsWithNilOptionalField_failedWithEqFilter() async {
let post = Post(title: UUID().uuidString, content: UUID().uuidString, createdAt: .now())
do {
await setUp(withModels: TestModelRegistration())
try await startAmplifyAndWaitForSync()

try await Amplify.DataStore.save(post)
let posts = try await Amplify.DataStore.query(
Post.self,
where: Post.keys.draft.eq("null")
.and(Post.keys.createdAt.ge(post.createdAt))
)
XCTAssertFalse(posts.map(\.id).contains(post.id))
} catch {
XCTFail("eq filter should not include records with optinal field .none")
}
}

/**
- Given: DataStore with Post schema and optional field 'draft'
- When:
- create a new post with optional field 'draft' value .none
- Then:
- query Posts with filter {attributeExists : false} should include the post
*/
func test_listModelsWithNilOptionalField_successWithAttributeExistsFilter() async {
let post = Post(title: UUID().uuidString, content: UUID().uuidString, createdAt: .now())
do {
await setUp(withModels: TestModelRegistration())
try await startAmplifyAndWaitForSync()

try await Amplify.DataStore.save(post)
let posts = try await Amplify.DataStore.query(
Post.self,
where: Post.keys.draft.attributeExists(false)
.and(Post.keys.createdAt.ge(post.createdAt))
)
XCTAssertTrue(posts.map(\.id).contains(post.id))
} catch {
XCTFail("attributeExists filter should include records with optinal field .none")
}
}

// MARK: - Helpers

private func sleepMill(_ milliseconds: UInt64) async throws {
Expand Down

0 comments on commit 31d507d

Please sign in to comment.