Hello Suresh
There some things that looks unclear for me in the last patch... Can you
explain the points below ?
Thanks,
Boris
suresh <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
+
+ * DataTable.cs: Reworked DataTable.LoadDataRow method after
+ regressions caused by Index redesign.
+ * DataRow.cs: Load : reworked.
+
2005-05-25 Konstantin Triger <[EMAIL PROTECTED]>
* 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 <[EMAIL PROTECTED]>
+
+ * DataTableLoadRowTest.cs: Added additional cases for
+ AutoIncrementTest to gauge any side effect with auto
+ incrementing in case of upsert.
+
2005-05-20 Sureshkumar T <[EMAIL PROTECTED]>
* 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@lists.ximian.com
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
