Bug 20925

Summary: Adding an expression (computed) DataColumn to a DataTable does not evaluate the expression for the IsNull method
Product: [Mono] Class Libraries Reporter: Matthew Leibowitz <matthew.leibowitz>
Component: System.DataAssignee: Bugzilla <bugzilla>
Severity: major CC: dominique, kumpera, masafa, mono-bugs+mono
Priority: ---    
Version: unspecified   
Target Milestone: Untriaged   
Hardware: All   
OS: All   
Tags: Is this bug a regression?: ---
Last known good build:

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 ?