Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Changing the database cursor to return default for DBNull #4070

Merged
merged 3 commits into from
Aug 7, 2019
Merged

Changing the database cursor to return default for DBNull #4070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4648e01
Changing the database cursor to return default for DBNull
tannergooding Aug 6, 2019
8312c03
Fixing float/double to be NaN for null values in the database loader.
tannergooding Aug 6, 2019
f8fac1c
Fixing the DatabaseLoaderTests mock to return false for IsDBNull
tannergooding Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions src/Microsoft.ML.Experimental/DataLoadSave/Database/DatabaseLoaderCursor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ private Delegate CreateGetterDelegate<TValue>(int col)
private ValueGetter<bool> CreateBooleanGetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref bool value) => value = DataReader.GetBoolean(columnIndex);
return (ref bool value) => value = DataReader.IsDBNull(columnIndex) ? default : DataReader.GetBoolean(columnIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think blindly turning null into default is really the correct thing to do here.

I wonder if we should have optional behaviors that the user can opt into. Potentially something like:

  1. (default behavior) throw on nulls so the user knows they have to make some decision.
  2. Turn null into default.
  3. Convert nullable integer columns into float/double, and use NaN to designate null values. They can then use the Replace N/A transforms available to them in the rest of the pipeline.
  4. The user can always change their schema (either inserting the data into a different table and replacing null as appropriate, creating a special stored proc or SELECT statement to do the null conversion) as an option to get around the exception as well.

See @TomFinley's comments at #673 (comment) for more thoughts here.

@codemzs @ebarsoumMS - any thoughts on the appropriate behavior here?

}

private ValueGetter<byte> CreateByteGetterDelegate(ColInfo colInfo)
Expand All @@ -254,61 +254,61 @@ private ValueGetter<DateTime> CreateDateTimeGetterDelegate(ColInfo colInfo)
private ValueGetter<double> CreateDoubleGetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref double value) => value = DataReader.GetDouble(columnIndex);
return (ref double value) => value = DataReader.IsDBNull(columnIndex) ? double.NaN : DataReader.GetDouble(columnIndex);
}

private ValueGetter<short> CreateInt16GetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref short value) => value = DataReader.GetInt16(columnIndex);
return (ref short value) => value = DataReader.IsDBNull(columnIndex) ? default : DataReader.GetInt16(columnIndex);
}

private ValueGetter<int> CreateInt32GetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref int value) => value = DataReader.GetInt32(columnIndex);
return (ref int value) => value = DataReader.IsDBNull(columnIndex) ? default : DataReader.GetInt32(columnIndex);
}

private ValueGetter<long> CreateInt64GetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref long value) => value = DataReader.GetInt64(columnIndex);
return (ref long value) => value = DataReader.IsDBNull(columnIndex) ? default : DataReader.GetInt64(columnIndex);
}

private ValueGetter<sbyte> CreateSByteGetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref sbyte value) => value = (sbyte)DataReader.GetByte(columnIndex);
return (ref sbyte value) => value = DataReader.IsDBNull(columnIndex) ? default : (sbyte)DataReader.GetByte(columnIndex);
}

private ValueGetter<float> CreateSingleGetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref float value) => value = DataReader.GetFloat(columnIndex);
return (ref float value) => value = DataReader.IsDBNull(columnIndex) ? float.NaN : DataReader.GetFloat(columnIndex);
}

private ValueGetter<ReadOnlyMemory<char>> CreateStringGetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref ReadOnlyMemory<char> value) => value = DataReader.GetString(columnIndex).AsMemory();
return (ref ReadOnlyMemory<char> value) => value = DataReader.IsDBNull(columnIndex) ? default : DataReader.GetString(columnIndex).AsMemory();
}

private ValueGetter<ushort> CreateUInt16GetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref ushort value) => value = (ushort)DataReader.GetInt16(columnIndex);
return (ref ushort value) => value = DataReader.IsDBNull(columnIndex) ? default : (ushort)DataReader.GetInt16(columnIndex);
}

private ValueGetter<uint> CreateUInt32GetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref uint value) => value = (uint)DataReader.GetInt32(columnIndex);
return (ref uint value) => value = DataReader.IsDBNull(columnIndex) ? default : (uint)DataReader.GetInt32(columnIndex);
}

private ValueGetter<ulong> CreateUInt64GetterDelegate(ColInfo colInfo)
{
int columnIndex = GetColumnIndex(colInfo);
return (ref ulong value) => value = (ulong)DataReader.GetInt64(columnIndex);
return (ref ulong value) => value = DataReader.IsDBNull(columnIndex) ? default : (ulong)DataReader.GetInt64(columnIndex);
}

private int GetColumnIndex(ColInfo colInfo)
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.ML.Tests/DatabaseLoaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ public override int GetOrdinal(string name)

public override int GetValues(object[] values) => throw new NotImplementedException();

public override bool IsDBNull(int ordinal) => throw new NotImplementedException();
public override bool IsDBNull(int ordinal) => false;

public override bool NextResult() => throw new NotImplementedException();

Expand Down