Skip to content

Commit eab6499

Browse files
grouegroue
committed
More support for inclusion checks with explicit collation
This commit addresses #1094 The position of the `collating` method is now irrelevant in all those cases: // name COLLATE NOCASE IN ('foo', 'bar') ["foo", "bar"].contains(Column("name")).collating(.nocase) ["foo", "bar"].contains(Column("name").collating(.nocase)) // name BETWEEN 'foo' AND 'bar' COLLATE NOCASE ("foo"..."bar").contains(Column("name")).collating(.nocase) ("foo"..."bar").contains(Column("name").collating(.nocase)) // (name >= 'foo' COLLATE NOCASE) AND (name < 'bar' COLLATE NOCASE) ("foo"..<"bar").contains(Column("name")).collating(.nocase) ("foo"..<"bar").contains(Column("name").collating(.nocase))
1 parent 21681e9 commit eab6499

File tree

2 files changed

+121
-4
lines changed

2 files changed

+121
-4
lines changed

‎GRDB/QueryInterface/SQL/SQLExpression.swift‎

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,17 @@ extension SQLExpression {
452452
if case let .collated(expression, collationName) = expression.impl {
453453
// Prefer: expression BETWEEN lowerBound AND upperBound COLLATE collation
454454
// over: (expression COLLATE collation) BETWEEN lowerBound AND upperBound
455+
//
456+
// This transformation was introduced in GRDB v0.42.0, for the first
457+
// release of the query interface:
458+
// https://github.com/groue/GRDB.swift/blob/3b3cb6bdecdfaac6e3d55bb7ecccf22f2749140f/GRDB/FetchRequest/SQLSupport/Collation.swift#L224-L239
459+
// The commit is a big squash, and we've lost the original intent.
460+
// It is likely just an SQL aesthetic preference of mine.
461+
//
462+
// According to https://www.sqlite.org/datatype3.html#assigning_collating_sequences_from_sql
463+
// this rewriting should not have any functional impact. Yet if any
464+
// user complains eventually, we should just remove this rewriting
465+
// rule without any resistance.
455466
return collated(between(
456467
expression: expression,
457468
lowerBound: lowerBound,
@@ -473,6 +484,8 @@ extension SQLExpression {
473484
/// <lhs> <= <rhs>
474485
/// <lhs> LIKE <rhs>
475486
static func binary(_ op: BinaryOperator, _ lhs: SQLExpression, _ rhs: SQLExpression) -> Self {
487+
// See `between(expression:lowerBound:upperBound:isNegated:)` for some
488+
// explanation of these rewriting rules.
476489
if case let .collated(lhs, collationName) = lhs.impl {
477490
// Prefer: lhs <= rhs COLLATE collation
478491
// over: (lhs COLLATE collation) <= rhs
@@ -551,8 +564,6 @@ extension SQLExpression {
551564
///
552565
/// See also `SQLCollection.contains(_:)`.
553566
static func `in`(_ expression: SQLExpression, _ collection: SQLCollection, isNegated: Bool = false) -> Self {
554-
// TODO: check if we should perform the same handling of collation as in `between`.
555-
// This could help https://github.com/groue/GRDB.swift/issues/1094#issuecomment-954043388
556567
self.init(impl: .in(expression, collection, isNegated: isNegated))
557568
}
558569

@@ -572,6 +583,8 @@ extension SQLExpression {
572583
///
573584
/// See also `SQLExpression.equal(_:_:)`.
574585
static func compare(_ op: EqualityOperator, _ lhs: SQLExpression, _ rhs: SQLExpression) -> Self {
586+
// See `between(expression:lowerBound:upperBound:isNegated:)` for some
587+
// explanation of these rewriting rules.
575588
if case let .collated(lhs, collationName) = lhs.impl {
576589
// Prefer: lhs = rhs COLLATE collation
577590
// over: (lhs COLLATE collation) = rhs
@@ -620,7 +633,57 @@ extension SQLExpression {
620633
///
621634
/// <expression> COLLATE <collation>
622635
static func collated(_ expression: SQLExpression, _ collationName: Database.CollationName) -> Self {
623-
self.init(impl: .collated(expression, collationName))
636+
switch expression.impl {
637+
case let .in(expression, collection, isNegated: isNegated):
638+
// According to https://www.sqlite.org/datatype3.html#assigning_collating_sequences_from_sql
639+
//
640+
// > The collating sequence used for expressions of the form
641+
// > "x IN (y, z, ...)" is the collating sequence of x. If an
642+
// > explicit collating sequence is required on an IN operator it
643+
// > should be applied to the left operand, like this:
644+
// > "x COLLATE nocase IN (y,z, ...)".
645+
//
646+
// Indeed:
647+
//
648+
// $ sqlite3
649+
// SQLite version 3.32.3 2020-06-18 14:16:19
650+
// sqlite> SELECT 'a' IN ('A') COLLATE NOCASE;
651+
// 0
652+
// sqlite> SELECT ('a' COLLATE NOCASE) IN ('A');
653+
// 1
654+
//
655+
// Conclusion: "x IN (y,z, ...) COLLATE nocase" can not match the
656+
// user intent. We could fatal error. Or warn. Or just make it work:
657+
//
658+
// Prefer: (expression COLLATE collation) IN (...)
659+
// over: expression IN (...) COLLATE collation
660+
return .in(.collated(expression, collationName), collection, isNegated: isNegated)
661+
662+
case let .associativeBinary(op, expressions):
663+
// The expression rewrite performed for the `IN` operator above
664+
// allows the user to have the following Swift code match the intent:
665+
//
666+
// // name COLLATE NOCASE IN ('foo', 'bar')
667+
// ["foo", "bar"].contains(Column("name")).collating(.nocase)
668+
// ["foo", "bar"].contains(Column("name").collating(.nocase))
669+
//
670+
// The BETWEEN case is supported as well (see
671+
// `between(expression:lowerBound:upperBound:isNegated:)`):
672+
//
673+
// // name BETWEEN 'foo' AND 'bar' COLLATE NOCASE
674+
// ("foo"..."bar").contains(Column("name")).collating(.nocase)
675+
// ("foo"..."bar").contains(Column("name").collating(.nocase))
676+
//
677+
// We just miss support for non-closed ranges:
678+
//
679+
// // (name >= 'foo' COLLATE NOCASE) AND (name < 'bar' COLLATE NOCASE)
680+
// ("foo"..<"bar").contains(Column("name")).collating(.nocase)
681+
// ("foo"..<"bar").contains(Column("name").collating(.nocase))
682+
return .associativeBinary(op, expressions.map { $0.collating(collationName) })
683+
684+
default:
685+
return self.init(impl: .collated(expression, collationName))
686+
}
624687
}
625688

626689
// MARK: Functions

‎Tests/GRDBTests/QueryInterfaceExpressionsTests.swift‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class QueryInterfaceExpressionsTests: GRDBTestCase {
162162
"SELECT * FROM \"readers\" WHERE \"name\" NOT BETWEEN 'A' AND 'z'")
163163
}
164164

165-
func testContainsWithCollation() throws {
165+
func testContainsCollated() throws {
166166
let dbQueue = try makeDatabaseQueue()
167167

168168
try dbQueue.read { db in
@@ -216,6 +216,60 @@ class QueryInterfaceExpressionsTests: GRDBTestCase {
216216
"SELECT * FROM \"readers\" WHERE (\"name\" >= 'A' COLLATE NOCASE) AND (\"name\" < 'z' COLLATE NOCASE)")
217217
}
218218

219+
func testCollatedContains() throws {
220+
let dbQueue = try makeDatabaseQueue()
221+
222+
try dbQueue.read { db in
223+
// Reminder of the SQLite behavior
224+
// https://sqlite.org/datatype3.html#assigning_collating_sequences_from_sql
225+
// > If an explicit collating sequence is required on an IN operator
226+
// > it should be applied to the left operand, like this:
227+
// > "x COLLATE nocase IN (y,z, ...)".
228+
try XCTAssertFalse(Bool.fetchOne(db, sql: "SELECT 'arthur' IN ('ARTHUR') COLLATE NOCASE")!)
229+
try XCTAssertTrue(Bool.fetchOne(db, sql: "SELECT 'arthur' COLLATE NOCASE IN ('ARTHUR')")!)
230+
}
231+
232+
// Array.contains(): IN operator
233+
XCTAssertEqual(
234+
sql(dbQueue, tableRequest.filter(["arthur", "barbara"].contains(Col.name).collating(.nocase))),
235+
"SELECT * FROM \"readers\" WHERE (\"name\" COLLATE NOCASE) IN ('arthur', 'barbara')")
236+
237+
XCTAssertEqual(
238+
sql(dbQueue, tableRequest.filter(!["arthur", "barbara"].contains(Col.name).collating(.nocase))),
239+
"SELECT * FROM \"readers\" WHERE (\"name\" COLLATE NOCASE) NOT IN ('arthur', 'barbara')")
240+
241+
XCTAssertEqual(
242+
sql(dbQueue, tableRequest.filter((["arthur", "barbara"] as [SQLExpressible]).contains(Col.name).collating(.nocase))),
243+
"SELECT * FROM \"readers\" WHERE (\"name\" COLLATE NOCASE) IN ('arthur', 'barbara')")
244+
245+
// Sequence.contains(): IN operator
246+
XCTAssertEqual(
247+
sql(dbQueue, tableRequest.filter(AnySequence(["arthur", "barbara"]).contains(Col.name).collating(.nocase))),
248+
"SELECT * FROM \"readers\" WHERE (\"name\" COLLATE NOCASE) IN ('arthur', 'barbara')")
249+
250+
// Sequence.contains(): = operator
251+
XCTAssertEqual(
252+
sql(dbQueue, tableRequest.filter(AnySequence([Col.name]).contains(Col.name).collating(.nocase))),
253+
"SELECT * FROM \"readers\" WHERE \"name\" = \"name\" COLLATE NOCASE")
254+
255+
// Sequence.contains(): false
256+
XCTAssertEqual(
257+
sql(dbQueue, tableRequest.filter(EmptyCollection<Int>().contains(Col.name).collating(.nocase))),
258+
"SELECT * FROM \"readers\" WHERE 0 COLLATE NOCASE")
259+
260+
// ClosedInterval: BETWEEN operator
261+
let closedInterval = "A"..."z"
262+
XCTAssertEqual(
263+
sql(dbQueue, tableRequest.filter(closedInterval.contains(Col.name).collating(.nocase))),
264+
"SELECT * FROM \"readers\" WHERE \"name\" BETWEEN 'A' AND 'z' COLLATE NOCASE")
265+
266+
// HalfOpenInterval: min <= x < max
267+
let halfOpenInterval = "A"..<"z"
268+
XCTAssertEqual(
269+
sql(dbQueue, tableRequest.filter(halfOpenInterval.contains(Col.name).collating(.nocase))),
270+
"SELECT * FROM \"readers\" WHERE (\"name\" >= 'A' COLLATE NOCASE) AND (\"name\" < 'z' COLLATE NOCASE)")
271+
}
272+
219273
func testSubqueryContains() throws {
220274
let dbQueue = try makeDatabaseQueue()
221275

0 commit comments

Comments
 (0)