Sqlite: Empty string init and coalescing for various properties #20172
Conversation
roji
changed the title
| private ConnectionState _state; | ||
| private sqlite3 _db; | ||
| private bool _extensionsEnabled; | ||
|
|
||
| private static readonly SqliteConnectionStringBuilder _emptyConnectionOptions = new SqliteConnectionStringBuilder(); |
There was a problem hiding this comment.
This seems dangerous since SqliteConnectionStringBuilder is mutable. I assume we never mutate it?
There was a problem hiding this comment.
Yep, I did a check and your codebase seems to be only reading from it (which seems like a good thing - mutable by the user, not by the driver). Are you good with this?
There was a problem hiding this comment.
SqlClient splits it into a mutable and an immutable type (for perf mostly). But I was lazy.
I'm good with it
|
|
||
| Assert.Equal(Resources.OpenRequiresSetConnectionString, ex.Message); | ||
| using var connection = new SqliteConnection(); | ||
| connection.Open(); |
There was a problem hiding this comment.
Added a SELECT 1 after this, that's what you meant right? Just to note that there are various other Open tests which don't do this.
There was a problem hiding this comment.
No, I just mean that most people will have no idea what this does and will be confused when their database is deleted when the connection is closed.
The test is fine
roji
force-pushed
the
SqliteEmptyStrings
branch
from
88b3f4e
to
2a3e1d5
Compare
Closes #20171