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

[Veerapuram Varadhan]

On Wed, 2008-07-09 at 09:39 +0200, Gert Driesen wrote:
> 
> --------------------------------------------------
> 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).
> 
Yes, I agree with you.  Please add the relevant tests as well. 

TIA,

V. Varadhan

> 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
> > 
>