Add a CreateSpanCollation method on SqliteConnection

[hahn-kev]

closes #35236

Adds a new method to SqliteConnection which enables creating allocation free collation methods. Here's an example of using it:

var compareInfo = cultureProvider.GetCompareInfo(writingSystem);
connection.CreateSpanCollation(
    writingSystem,
    compareInfo,
    static (compareInfo, x, y) => compareInfo.Compare(x, y, CompareOptions.IgnoreCase)
);

• I've read the guidelines for contributing and seen the walkthrough
• I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
• The code builds and tests pass locally (also verified by our automated build checks)
• Commit messages follow this format:

        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber

• Tests for the changes have been added (for bug fixes / features)
• Code follows the same patterns and style as existing code in this repo

There are no tests for the existing CreateCollation method, so it's unclear how or even if this new version should be tested also.

[roji]

See some comments. Also, this PR lacks tests.

[roji]

Should state be generic (T) instead of object?

[roji]

Why have in modifiers here? ReadOnlySpan is very small, so passing it in by reference doesn't seem to make sense, and in fact will make things slower, no?

[hahn-kev]

I'm not sure, I was copying Func, but looking at methods in CompareInfo none of them use the in modifier either so I'll drop it here.

[roji]

Do we need to have the special new name CreateSpanCollation rather than just an overload of CreateCollation?

[hahn-kev]

That would make this a breaking change because now something like con.CreateCollation("name", null, (t, s1, s2) => s1.Length - s2.Length) is ambiguous now. I'm not sure the policy on those kinds of breaking changes so that's why I didn't use the same name.

[roji]

This looks unsafe: stackalloc should never be used with an unbounded size. Unless I'm missing something, a huge string in the database could cause an arbitrary stack allocation and potentially cause a stack overflow, no?

Regardless, this copies/decodes the SQLite-provided ReadOnlySpan<byte> to ReadOnlySpan<char>; the point of this PR is to provide better performance, so that's not great. You should be able to simply reinterpret-cast using MemoryMarshal.Cast<byte, char>(), but need to check that the memory is aligned first. I'm assuming that SQLite-provided memory will generally be aligned (this needs to be checked); if that's the case, we can fall back to inefficient copying for unaligned memory (if that's a rare case).

[hahn-kev]

I think you're right about the size issue. I'm not sure if there's an existing pattern but it seems like we should avoid a stack allocation if the size is over some limit? Not sure what that would be though.

As for copying, that's required as a char is utf16 and a byte is only 8 bits, so we can't just reinterpret a byte as a char because they have different sizes.

That said there's an argument for this whole method to use ReadOnlySpan<byte> and not ReadOnlySpan<char>, or we could make both? I'm not sure, it's not very easy to handle utf8 strings in dotnet and if someone is going down to that level then they can just call sqlite3_create_collation themselves. Though it would be pretty easy for CreateCollation char to be implemented on top of the utf8 version.

[hahn-kev]

@roji, resolved some comments.

Sorry I didn't write tests at first, the test project wasn't loaded so I didn't find the tests which referenced CreateCollation. I've now copied the existing collation tests and rewritten them for the span version.