[Mono-devel-list] Re: [Mono-patches] r44988 - in trunk/mcs/class/System.Data: System.Data Test/System.Data
Boris Kirzner
borisk at mainsoft.com
Thu May 26 08:02:10 EDT 2005
Hello Suresh
There some things that looks unclear for me in the last patch... Can you
explain the points below ?
Thanks,
Boris
suresh <sthangavel at novell.com> wrote:
>Author: suresh
>Date: 2005-05-25 06:57:45 -0400 (Wed, 25 May 2005)
>New Revision: 44988
>
>Modified:
> trunk/mcs/class/System.Data/System.Data/ChangeLog
> trunk/mcs/class/System.Data/System.Data/DataRow.cs
> trunk/mcs/class/System.Data/System.Data/DataTable.cs
> trunk/mcs/class/System.Data/Test/System.Data/ChangeLog
> trunk/mcs/class/System.Data/Test/System.Data/DataTableLoadRowTest.cs
>Log:
> * System.Data/DataTable.cs: Reworked DataTable.LoadDataRow method after
> regressions caused by Index redesign.
> * System.Data/DataRow.cs: Load : reworked.
>
> * Test/System.Data/DataTableLoadRowTest.cs: Added additional cases for
> AutoIncrementTest to gauge any side effect with auto
> incrementing in case of upsert.
>
>
>
>Modified: trunk/mcs/class/System.Data/System.Data/ChangeLog
>===================================================================
>--- trunk/mcs/class/System.Data/System.Data/ChangeLog 2005-05-25 09:55:24 UTC (rev 44987)
>+++ trunk/mcs/class/System.Data/System.Data/ChangeLog 2005-05-25 10:57:45 UTC (rev 44988)
>@@ -1,3 +1,9 @@
>+2005-05-25 Sureshkumar T <tsureshkumar at novell.com>
>+
>+ * DataTable.cs: Reworked DataTable.LoadDataRow method after
>+ regressions caused by Index redesign.
>+ * DataRow.cs: Load : reworked.
>+
> 2005-05-25 Konstantin Triger <kostat at mainsoft.com>
>
> * ISafeDataRecord.cs: Added GetDateTimeSafe method, the interface was made derived from IDataRecord
>
>Modified: trunk/mcs/class/System.Data/System.Data/DataRow.cs
>===================================================================
>--- trunk/mcs/class/System.Data/System.Data/DataRow.cs 2005-05-25 09:55:24 UTC (rev 44987)
>+++ trunk/mcs/class/System.Data/System.Data/DataRow.cs 2005-05-25 10:57:45 UTC (rev 44988)
>@@ -590,14 +590,14 @@
> _table.ChangingDataRow (this, DataRowAction.Commit);
> CheckChildRows(DataRowAction.Commit);
> switch (RowState) {
>- case DataRowState.Unchanged:
>+ case DataRowState.Unchanged:
> return;
> case DataRowState.Added:
> case DataRowState.Modified:
>- if (Original >= 0) {
>- Table.RecordCache.DisposeRecord(Original);
>- }
>- Original = Current;
>+ if (Original >= 0) {
>+ Table.RecordCache.DisposeRecord(Original);
>+ }
>+ Original = Current;
> break;
> case DataRowState.Deleted:
> _table.Rows.RemoveInternal (this);
>@@ -1566,57 +1566,65 @@
> /// mentioned in the DataTable.Load (IDataReader, LoadOption) method.
> /// </summary>
> [MonoTODO ("Raise necessary Events")]
>- internal void Load (object [] values, LoadOption loadOption, bool is_new)
>+ internal void Load (object [] values, LoadOption loadOption)
> {
>+ Index index = null;
> DataRowAction action = DataRowAction.Change;
>
>- int temp = Table.RecordCache.NewRecord ();
>- for (int i = 0 ; i < Table.Columns.Count; i++)
>- SetValue (i, values [i], temp);
>+ int temp = -1;
>
>- if (is_new) { // new row
>- if (HasVersion (DataRowVersion.Proposed) || RowState == DataRowState.Detached)
>- Proposed = temp;
>- else
>- Current = temp;
>- return;
>- }
>-
> if (loadOption == LoadOption.OverwriteChanges
> || (loadOption == LoadOption.PreserveChanges
> && RowState == DataRowState.Unchanged)) {
>+ temp = Table.CreateRecord (values);
>+ if (HasVersion (DataRowVersion.Original) && Current != Original)
>+ Table.RecordCache.DisposeRecord (Original);
> Original = temp;
>- if (HasVersion (DataRowVersion.Proposed))
>- Proposed = temp;
>- else
>- Current = temp;
>+ // update the pk index
>+ index = Table.GetIndex(Table.PrimaryKey,null,DataViewRowState.None,null,false);
>+ if (index != null)
>+ index.Update (this, temp);
>+
>+ if (HasVersion (DataRowVersion.Current))
>+ Table.RecordCache.DisposeRecord (Current);
>+ Current = temp;
>
>
Is there some special reason for first updating Original version, then
updating an index, and then updating the Current version? Since index
built on RowState.None, it uses Default (and not updated) value in index
update.
> action = DataRowAction.ChangeCurrentAndOriginal;
> return;
> }
>
> if (loadOption == LoadOption.PreserveChanges) {
>- if (RowState != DataRowState.Deleted) {
>- Original = temp;
>- action = DataRowAction.ChangeOriginal;
>- }
>+ if (RowState == DataRowState.Deleted)
>+ return;
>+ temp = Table.CreateRecord (values);
>+ if (HasVersion (DataRowVersion.Original) && Current != Original)
>+ Table.RecordCache.DisposeRecord (Original);
>+ Original = temp;
>+ action = DataRowAction.ChangeOriginal;
> return;
> }
>
>- bool not_used = true;
> // Upsert
>
>
The following code runs also for loadOption other that Upset (
PreserveCurrentValues for example). Is this correct?
> if (RowState != DataRowState.Deleted) {
>- int index = HasVersion (DataRowVersion.Proposed) ? _proposed : _current;
>- if (Table.CompareRecords (index, temp) != 0) {
>- if (HasVersion (DataRowVersion.Proposed))
>- Proposed = temp;
>- else
>- Current = temp;
>- not_used = false;
>- }
>+ int rindex = HasVersion (DataRowVersion.Proposed) ? Proposed : Current;
>+ temp = Table.CreateRecord (values);
>+ if (Table.CompareRecords (rindex, temp) != 0) {
>+ if (HasVersion (DataRowVersion.Proposed)) {
>+ Table.RecordCache.DisposeRecord (Proposed);
>+ Proposed = -1;
>+ }
>+
>+ // update the pk index
>+ index = Table.GetIndex(Table.PrimaryKey,null,DataViewRowState.None,null,false);
>+ if (index != null)
>+ index.Update (this, temp);
>+
>+ if (Original != Current)
>+ Table.RecordCache.DisposeRecord (Current);
>+ Current = temp;
>+ } else
>+ Table.RecordCache.DisposeRecord (temp);
> }
>-
>- if (not_used)
>- Table.RecordCache.DisposeRecord (temp);
>+
> }
> #endif // NET_2_0
> }
>
>Modified: trunk/mcs/class/System.Data/System.Data/DataTable.cs
>===================================================================
>--- trunk/mcs/class/System.Data/System.Data/DataTable.cs 2005-05-25 09:55:24 UTC (rev 44987)
>+++ trunk/mcs/class/System.Data/System.Data/DataTable.cs 2005-05-25 10:57:45 UTC (rev 44988)
>@@ -1199,31 +1199,45 @@
> public DataRow LoadDataRow (object [] values, LoadOption loadOption)
> {
> DataRow row = null;
>- bool new_row = false;
>
> // Find Data DataRow
> if (this.PrimaryKey.Length > 0) {
>- int newRecord = CreateRecord(values);
>- try {
>- Index index = GetIndex(PrimaryKey,null,DataViewRowState.OriginalRows,null,false);
>- int existingRecord = index.Find(newRecord);
>+ Index index = FindIndex (PrimaryKey,null,DataViewRowState.OriginalRows,null);
>+ if (index == null)
>+ index = new Index (new Key(this, PrimaryKey,null,DataViewRowState.OriginalRows,null));
>
>
This index is used only for lookup - am I right?
>+
>+ object [] keys = new object [PrimaryKey.Length];
>+ for (int i=0; i < PrimaryKey.Length; i++)
>+ keys [i] = values [PrimaryKey [i].Ordinal];
>
>
This assignment will raise an exception if PrimaryKey[i].Ordinal exeeds
values.Length. Should probably check for this and raise the
corresponding exception (the row can not be added if not all the primary
keya values exists and the missing PK columns are not autoincrement).
>+
>+ int existingRecord = index.Find(keys);
>+ if (existingRecord >= 0)
>+ row = RecordCache[existingRecord];
>+ else {
>+ existingRecord = _primaryKeyConstraint.Index.Find(keys);
> if (existingRecord >= 0)
> row = RecordCache[existingRecord];
>- else {
>- existingRecord = _primaryKeyConstraint.Index.Find(newRecord);
>- if (existingRecord >= 0)
>- row = RecordCache[existingRecord];
>- }
> }
>- finally {
>- RecordCache.DisposeRecord(newRecord);
>- }
> }
>
> // If not found, add new row
>- if (row == null) {
>- row = this.NewRow ();
>- new_row = true;
>+ if (row == null
>+ || (row.RowState == DataRowState.Deleted
>+ && loadOption == LoadOption.Upsert)) {
>
>
According to MSDN Upsert should preserve an Original version. Are you
shure Deleted row always misses an Originlal version? If no, it should
be preserved.
>+ row = NewNotInitializedRow ();
>+ row.ImportRecord (CreateRecord(values));
>+
>+ if (EnforceConstraints)
>+ // we have to check that the new row doesn't colide with existing row
>+ Rows.ValidateDataRowInternal(row); // this adds to index ;-)
>+
>+ Rows.AddInternal(row);
>+
>+ if (loadOption == LoadOption.OverwriteChanges ||
>+ loadOption == LoadOption.PreserveChanges) {
>+ row.AcceptChanges ();
>+ }
>+ return row;
> }
>
> bool deleted = row.RowState == DataRowState.Deleted;
>@@ -1231,21 +1245,8 @@
> if (deleted && loadOption == LoadOption.OverwriteChanges)
> row.RejectChanges ();
>
>- row.Load (values, loadOption, new_row);
>+ row.Load (values, loadOption);
>
>- if (deleted && loadOption == LoadOption.Upsert) {
>- row = this.NewRow ();
>- row.Load (values, loadOption, new_row = true);
>- }
>-
>- if (new_row) {
>- this.Rows.Add (row);
>- if (loadOption == LoadOption.OverwriteChanges ||
>- loadOption == LoadOption.PreserveChanges) {
>- row.AcceptChanges ();
>- }
>- }
>-
> return row;
> }
>
>
>Modified: trunk/mcs/class/System.Data/Test/System.Data/ChangeLog
>===================================================================
>--- trunk/mcs/class/System.Data/Test/System.Data/ChangeLog 2005-05-25 09:55:24 UTC (rev 44987)
>+++ trunk/mcs/class/System.Data/Test/System.Data/ChangeLog 2005-05-25 10:57:45 UTC (rev 44988)
>@@ -1,3 +1,9 @@
>+2005-05-25 Sureshkumar T <tsureshkumar at novell.com>
>+
>+ * DataTableLoadRowTest.cs: Added additional cases for
>+ AutoIncrementTest to gauge any side effect with auto
>+ incrementing in case of upsert.
>+
> 2005-05-20 Sureshkumar T <tsureshkumar at novell.com>
>
> * DataRowCollectionTest.cs: Added a test to check Rows.Add (values
>
>Modified: trunk/mcs/class/System.Data/Test/System.Data/DataTableLoadRowTest.cs
>===================================================================
>--- trunk/mcs/class/System.Data/Test/System.Data/DataTableLoadRowTest.cs 2005-05-25 09:55:24 UTC (rev 44987)
>+++ trunk/mcs/class/System.Data/Test/System.Data/DataTableLoadRowTest.cs 2005-05-25 10:57:45 UTC (rev 44988)
>@@ -271,6 +271,16 @@
> Assert.AreEqual (25, dt.Rows [3] [0], "#2 current should be ai");
> Assert.AreEqual (25, dt.Rows [3] [0, DataRowVersion.Original], "#3 original should be ai");
>
>+ dt.LoadDataRow (new object [] {25, 20, "mono test"}, LoadOption.Upsert);
>+ dt.LoadDataRow (new object [] {25, 20, "mono test 2"}, LoadOption.Upsert);
>+ dt.LoadDataRow (new object [] {null, 20, "mono test aaa"}, LoadOption.Upsert);
>+
>+ Assert.AreEqual (5, dt.Rows.Count, "#4 has not added a new row");
>+ Assert.AreEqual (25, dt.Rows [3] [0], "#5 current should be ai");
>+ Assert.AreEqual (25, dt.Rows [3] [0, DataRowVersion.Original], "#6 original should be ai");
>+
>+ Assert.AreEqual (30, dt.Rows [4] [0], "#7 current should be ai");
>+
> }
> }
> }
>
>_______________________________________________
>Mono-patches maillist - Mono-patches at lists.ximian.com
>http://lists.ximian.com/mailman/listinfo/mono-patches
>
>
More information about the Mono-devel-list
mailing list