Add SqliteConnection.CreateCollation method #340
Conversation
|
@AlexanderTaeschner, |
|
Per the contributing guidelines:
Let's work together on a design before looking at any code. Could you add your proposal to the issue? |
|
Re-opening. This is pretty close to the design we landed on. Would you mind updating it? |
|
Updated to new API spec in #19. |
| /// <param name="name">Name of the collation.</param> | ||
| /// <param name="comparison">Method that compares two strings.</param> | ||
| public virtual void CreateCollation(string name, Comparison<string> comparison) | ||
| => CreateSqlite3Collation(name, null, (_, s1, s2) => comparison(s1, s2)); |
There was a problem hiding this comment.
Personally, I'd just call CreateCollation<object>(name, null,... and inline CreateSqlite3Collation into the other overload.
| /// <param name="comparison">Method that compares two strings, using additional state.</param> | ||
| public virtual void CreateCollation<T>(string name, T state, Func<T, string, string, int> comparison) | ||
| { | ||
| var rc = raw.sqlite3_create_collation(_db, name, state, (v, s1, s2) => comparison((T)v, s1, s2)); |
There was a problem hiding this comment.
Oh, we'll need to throw if the connection isn't open.
There was a problem hiding this comment.
Added check and test case.
| /// </summary> | ||
| /// <param name="name">Name of the collation.</param> | ||
| /// <param name="comparison">Method that compares two strings.</param> | ||
| public virtual void CreateCollation(string name, Comparison<string> comparison) |
There was a problem hiding this comment.
Passing a null comparison will currently result in a NullReferenceException. We should either:
- Throw
ArgumentNullException - Support it by flowing
nullintosqlite3_create_collation(which removes the collation)
There was a problem hiding this comment.
We should probably validate name too if the native API gives its "misuse" error for null or empty values.
There was a problem hiding this comment.
Opted for option 2 and added test.
| () => connection.ExecuteScalar<long>("SELECT 'Νικοσ' = 'ΝΙΚΟΣ' COLLATE MY_NOCASE;")); | ||
|
|
||
| Assert.Equal(raw.SQLITE_ERROR, ex.SqliteErrorCode); | ||
| Assert.Contains("no such collation sequence", ex.Message); |
There was a problem hiding this comment.
Don't assert the message. (In case SQLite ever decides to localize.)
There was a problem hiding this comment.
Actually, I think there's a PRAGMA you can use to see the registered collations. It might be better to use that.
There was a problem hiding this comment.
Removed assertion. Adding a complete ExecuteReader loop for analyzing the PRAGMA seems to be overly complex. Since sqlite version 3.16.0 there are table-valued functions for the PRAGMAs usable in SELECTs, but I don't think that we can rely on the use of such a new version.
There was a problem hiding this comment.
OT: Are table-valued functions supported in general?
| } | ||
|
|
||
| delegate_collation collation = comparison != null ? (v, s1, s2) => comparison((T)v, s1, s2) : (delegate_collation)null; | ||
| var rc = raw.sqlite3_create_collation(_db, name, state, collation); |
There was a problem hiding this comment.
Do we need to validate name?
There was a problem hiding this comment.
Added validation.
Addresses #19