jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
856 stars 145 forks source link

DbNull for string when doing Bulk Inserts #514

Closed JejoSwaks closed 2 months ago

JejoSwaks commented 2 months ago

Nullable properties in BulkInsert

I am trying to bulk insert a basic POCO that contains some string properties. These can, and may be NULL (null). However, when the field accessor is being used to write the CSV (using the Postgres driver in my case); I get a NullReferenceException (on the value.ToString() invocation). I could default all my string properties to string.Empty but that would be semantically incorrect.

I tried to circumvent this by wrapping string inside a DbNullable<T> with implicit operators to allow for the logic to properly compare to DbNull and to allow for implicit conversion (assignment) from T.

This works for null values (since the comparison overloads tell the logic to skip the property/field). For non-null values, I added a ToString() override to return the wrapped value. This does not work: the accessor that is being generated emits a ToString() callvirt since the database type is DbType.String. Obviously, the value that is to be compared to DbNull is now again null where I hoped it would be my DbNullable<T>

Either I am an idiot doing something wrong, or, going the wrong way about it. Or, this is a (known) problem (with a known solution). I've read through all the related topics on the wiki pages but couldn't find a solution or workaround.

Basically: I want to have a property that can compare to DbNull and also has a custom ToString() method.

Or - a an additional null check here in PostgreSQLInsightDbProvider.cs:

 object value = reader.GetValue(i);

 if (value != DBNull.Value)  <-- if (value != DBNull.Value && value != null) 
 {
     writer.Write(CsvQuote);
     writer.Write(_csvRegex.Replace(value.ToString(), CsvReplacement));
     writer.Write(CsvQuote);
 }
jonwagner commented 2 months ago

I don't think you're missing anything. Probably just a missed use case. The BulkCopy code has different code paths from sql calls.

Can you make a small PR that adds a test case to the Postgres tests? Then I can just step through and fix it.

jonwagner commented 2 months ago

You were right - just a null check. I'll tidy up a few other updates then push out a build.

jonwagner commented 2 months ago

This should be fixed in 8.0.1