[Mono-dev] [Mono-patches] r107145 - trunk/mcs/class/System.Data/System.Data

Wed Jul 9 03:39:14 EDT 2008

--------------------------------------------------
From: "Veerapuram Varadhan" <vvaradhan at novell.com>
Sent: Wednesday, July 09, 2008 8:42 AM
To: "Gert Driesen" <gert.driesen at telenet.be>
Cc: "'mono-devel-list'" <mono-devel-list at ximian.com>
Subject: Re: [Mono-dev][Mono-patches]	r107145	-trunk/mcs/class/System.Data/System.Data> Hi Gert,
>
> On Fri, 2008-07-04 at 21:35 +0200, Gert Driesen wrote:
>> Hey Veerapuram,
>>
>> I've attached a patch for the changes I mentioned. However, I found out 
>> that
>> on the 1.0 profile we should still allow the value to be set to null if 
>> the
>> datatype of the column is string. This matches the behavior before 
>> r107145.
>>
>> Let me know if it's ok to commit.
>>
> In case of 2.0 profile, in case of reference types, we have to set value
> to DBNull instead of NULL - which the patch is missing. Except that,
> patch looks good - please commit along with the mentioned changes.

Are you sure that we need to set it to DBNull? Do you have a test that shows 
this to be necessary?

At least one of the unit tests I shows that 
DataColumnChangeEventArgs.ProposedValue of the ColumnChanged/ColumnChanging 
events has value NULL (and not DBNull) on MS.NET 2.0.

My patch passed all unit tests, and I created a few connected tests to be 
sure I did not break anthing.

It's of course more important not to break existing application than to get 
some unit test to pass.

However, if not setting the value to DBNull indead breaks applications, then 
I'd like to have a test for this (to ensure we won't be breaking this in the 
future).

Gert

>> -----Original Message-----
>> From: mono-devel-list-bounces at lists.ximian.com
>> [mailto:mono-devel-list-bounces at lists.ximian.com] On Behalf Of Veerapuram
>> Varadhan
>> Sent: vrijdag 4 juli 2008 12:18
>> To: Gert Driesen
>> Cc: 'mono-devel-list'
>> Subject: Re: [Mono-dev] [Mono-patches] r107145 -
>> trunk/mcs/class/System.Data/System.Data
>>
>> Hi Gert,
>>
>> On Thu, 2008-07-03 at 18:19 +0200, Gert Driesen wrote:
>> > Hey Veerapuram,
>> >
>> > I don't think this change is correct. I've done some more tests, and 
>> > this
>> is
>> > the behavior I'm seeing:
>> >
>> > * on 1.0 profile, you are never allowed to set value to NULL.
>> > * on 2.0 profile, you are only allowed to set value to NULL if the 
>> > column
>> is
>> > backed by a reference type.
>> >
>> > I'll add unit tests for this to DataRowTest2.cs in a few minutes, and 
>> > mark
>> > them NotWorking.
>> >
>> > Let me know if you want me to submit a patch that changes our
>> implementation
>> > accordingly.
>> >
>> That would be great.  Feel free to submit tests and the patch.
>>
>> Thanks,
>>
>> V. Varadhan
>>
>> > Gert
>> >
>> > -----Original Message-----
>> > From: mono-patches-bounces at lists.ximian.com
>> > [mailto:mono-patches-bounces at lists.ximian.com] On Behalf Of Veerapuram
>> > Varadhan (vvaradhan at novell.com)
>> > Sent: donderdag 3 juli 2008 15:38
>> > To: mono-patches at lists.ximian.com; ximian.monolist at gmail.com;
>> > mono-svn-patches-garchive-20758 at googlegroups.com
>> > Subject: [Mono-patches] r107145 - 
>> > trunk/mcs/class/System.Data/System.Data
>> >
>> > Author: varadhan
>> > Date: 2008-07-03 09:38:09 -0400 (Thu, 03 Jul 2008)
>> > New Revision: 107145
>> >
>> > Modified:
>> >    trunk/mcs/class/System.Data/System.Data/ChangeLog
>> >    trunk/mcs/class/System.Data/System.Data/DataRow.cs
>> > Log:
>> > Use DBNull value instead of throwing an exception
>> >
>> >
>> > Modified: trunk/mcs/class/System.Data/System.Data/ChangeLog
>> > ===================================================================
>> > --- trunk/mcs/class/System.Data/System.Data/ChangeLog 2008-07-03 
>> > 13:37:14
>> > UTC (rev 107144)
>> > +++ trunk/mcs/class/System.Data/System.Data/ChangeLog 2008-07-03 
>> > 13:38:09
>> > UTC (rev 107145)
>> > @@ -1,3 +1,7 @@
>> > +2008-07-03  Marek Habersack  <mhabersack at novell.com>
>> > +
>> > + * DataRow.cs (this []): Use DBNull instead of throwing an exception
>> > +
>> >  2008-07-01  Rodrigo Kumpera  <rkumpera at novell.com>
>> >
>> >  * DataTable.cs: Kill some foreach loops.
>> >
>> > Modified: trunk/mcs/class/System.Data/System.Data/DataRow.cs
>> > ===================================================================
>> > --- trunk/mcs/class/System.Data/System.Data/DataRow.cs 2008-07-03
>> 13:37:14
>> > UTC (rev 107144)
>> > +++ trunk/mcs/class/System.Data/System.Data/DataRow.cs 2008-07-03
>> 13:38:09
>> > UTC (rev 107145)
>> > @@ -178,9 +178,8 @@
>> >  DataColumn column =
>> > _table.Columns[columnIndex];
>> >  _table.ChangingDataColumn (this, column,
>> > value);
>> >
>> > - if (value == null && column.DataType !=
>> > typeof(string)) {
>> > - throw new ArgumentException("Cannot
>> > set column " + column.ColumnName + " to be null, Please use DBNull
>> > instead");
>> > - }
>> > + if (value == null && column.DataType !=
>> > typeof(string))
>> > + value = DBNull.Value;
>> >  _rowChanged = true;
>> >
>> >  CheckValue (value, column);
>> >
>> > _______________________________________________
>> > Mono-patches maillist  -  Mono-patches at lists.ximian.com
>> > http://lists.ximian.com/mailman/listinfo/mono-patches
>> >
>>
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list at lists.ximian.com
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> 