Bug 20925 - Adding an expression (computed) DataColumn to a DataTable does not evaluate the expression for the IsNull method
Summary: Adding an expression (computed) DataColumn to a DataTable does not evaluate t...
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: System.Data ()
Version: unspecified
Hardware: All All
: --- major
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2014-06-26 19:05 UTC by Matthew Leibowitz
Modified: 2017-09-06 17:04 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Matthew Leibowitz 2014-06-26 19:05:04 UTC
When adding a new expression column to a table with existing rows, does not evaluate to the correct value for the IsNull method:

DataTable table = new DataTable ();

// add the row, with the value in the column
DataColumn staticColumn = table.Columns.Add ("static", typeof (string), null); // static
DataRow row = table.Rows.Add ("the value");
Assert.IsFalse (row.IsNull ("static"), "static null check failed");
Assert.AreEqual ("the value", row ["static"], "static value check failed");

// add the first derived column
DataColumn firstColumn = table.Columns.Add ("first", typeof (string), "static"); // first -> static
Assert.IsFalse (row.IsNull ("first"), "first level null check failed"); // <- this fails
Assert.AreEqual ("the value", row ["first"], "first level value check failed");
Assert.IsFalse (row.IsNull ("first"), "first level null check failed"); // <- if the first failure was ignored, this will actually succeed as the actual retrieval of the value (in the row above) updates the null status of the column.
Comment 1 Matthew Leibowitz 2014-06-26 19:06:25 UTC
Here is a fix:

public bool IsNull (DataColumn column, DataRowVersion version)
{
	// use the expresion if there is one
	if (column.Expression != String.Empty) {
		// FIXME: how does this handle 'version'?
		// TODO: Can we avoid the Eval each time by using the cached value?
		object o = column.CompiledExpression.Eval (this);
		return o == null && o == DBNull.Value;
	}

	return column.DataContainer.IsNull (IndexFromVersion (version));
}

Just want to run it through the system before creating a pull request... The last line is also the only current line.

If no one responds soon, then I will update and create a pull...
Comment 2 Matthew Leibowitz 2014-06-26 19:07:52 UTC
NOTE: the FIXME and TODO comments are also in another method:
    public object this [int columnIndex, DataRowVersion version]

This was there, so when one is fixed, this should be too.
Comment 3 Matthew Leibowitz 2014-06-26 19:09:41 UTC
Another note: this is a bug in the DataRowTest2.IsNull_ByName unit test, but there is a Console.WriteLine method call that effectively hides this bug.
Comment 5 Dominique NORMAND 2015-08-12 10:48:11 UTC
The fix is incorrect. It causes the method to always return false for columns that have an expression.
This is due to the use of && instead of ||

The line 
      return o == null && o == DBNull.Value; 
should be replaced with 
      return o == null || o == DBNull.Value;
Comment 6 Matthew Leibowitz 2015-08-21 18:48:26 UTC
Thanks for picking that up. I'll correct it.
Comment 7 Matthew Leibowitz 2015-08-24 23:47:49 UTC
This issue was fixed in master
Comment 8 Rodrigo Kumpera 2015-09-03 17:31:53 UTC
Hi Matthew,

Could you make a PR for this bug fix against mono 4.2 ?