On Wed, 2008-07-09 at 09:39 +0200, Gert Driesen wrote: > > -------------------------------------------------- > From: "Veerapuram Varadhan" <[EMAIL PROTECTED]> > Sent: Wednesday, July 09, 2008 8:42 AM > To: "Gert Driesen" <[EMAIL PROTECTED]> > Cc: "'mono-devel-list'" <[EMAIL PROTECTED]> > 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). > Yes, I agree with you. Please add the relevant tests as well.
TIA, V. Varadhan > Gert > > >> -----Original Message----- > >> From: [EMAIL PROTECTED] > >> [mailto:[EMAIL PROTECTED] 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: [EMAIL PROTECTED] > >> > [mailto:[EMAIL PROTECTED] On Behalf Of Veerapuram > >> > Varadhan ([EMAIL PROTECTED]) > >> > Sent: donderdag 3 juli 2008 15:38 > >> > To: [EMAIL PROTECTED]; [EMAIL PROTECTED]; > >> > [EMAIL PROTECTED] > >> > 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 <[EMAIL PROTECTED]> > >> > + > >> > + * DataRow.cs (this []): Use DBNull instead of throwing an exception > >> > + > >> > 2008-07-01 Rodrigo Kumpera <[EMAIL PROTECTED]> > >> > > >> > * 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 - [EMAIL PROTECTED] > >> > http://lists.ximian.com/mailman/listinfo/mono-patches > >> > > >> > >> _______________________________________________ > >> Mono-devel-list mailing list > >> Mono-devel-list@lists.ximian.com > >> http://lists.ximian.com/mailman/listinfo/mono-devel-list > > > > _______________________________________________ > > Mono-devel-list mailing list > > Mono-devel-list@lists.ximian.com > > http://lists.ximian.com/mailman/listinfo/mono-devel-list > > > _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list