* Cleanup of adding records to bufdatasets with indexes. Mantis #20514, patch by Lacak2.

git-svn-id: trunk@21023 -

packages/fcl-db/src/base/bufdataset.pas

@@ -951,11 +951,14 @@ begin
       PCurRecLinkItem[DblLinkIndex.IndNr].next := PCurRecLinkItem[0].next;
       PCurRecLinkItem[DblLinkIndex.IndNr].prior := PCurRecLinkItem[0].prior;
       end;
-    end;
+    end
+  else
+    // Empty dataset
+    Exit;
 
 // Set FirstRecBuf and FCurrentRecBuf
   DblLinkIndex.FFirstRecBuf:=Index0.FFirstRecBuf;
-  (FCurrentIndex as TDoubleLinkedBufIndex).FCurrentRecBuf:=DblLinkIndex.FFirstRecBuf;
+  DblLinkIndex.FCurrentRecBuf:=DblLinkIndex.FFirstRecBuf;
 // Link in the FLastRecBuf that belongs to this index
   PCurRecLinkItem[DblLinkIndex.IndNr].next:=DblLinkIndex.FLastRecBuf;
   DblLinkIndex.FLastRecBuf[DblLinkIndex.IndNr].prior:=PCurRecLinkItem;
@@ -975,7 +978,7 @@ begin
 // of as we finish dealing with them.
 
   p := DblLinkIndex.FFirstRecBuf;
-  DblLinkIndex.ffirstRecBuf := nil;
+  DblLinkIndex.FFirstRecBuf := nil;
   q := p;
   MergeAmount := 0;
 
@@ -1079,7 +1082,7 @@ begin
   Result:= True;
 end;
 
-function TCustomBufDataset.intAllocRecordBuffer: TRecordBuffer;
+function TCustomBufDataset.IntAllocRecordBuffer: TRecordBuffer;
 begin
   // Note: Only the internal buffers of TDataset provide bookmark information
   result := AllocMem(FRecordsize+sizeof(TBufRecLinkItem)*FMaxIndexesCount);
@@ -1203,7 +1206,7 @@ procedure TCustomBufDataset.InternalLast;
 begin
   FetchAll;
   with FCurrentIndex do
-  SetToLastRecord;
+    SetToLastRecord;
 end;
 
 function TDoubleLinkedBufIndex.GetCurrentRecord: TRecordBuffer;
@@ -1362,6 +1365,7 @@ procedure TDoubleLinkedBufIndex.InitialiseSpareRecord(const ASpareRecord : TReco
 begin
   FFirstRecBuf := pointer(ASpareRecord);
   FLastRecBuf := FFirstRecBuf;
+  FLastRecBuf[IndNr].prior:=nil;
   FLastRecBuf[IndNr].next:=FLastRecBuf;
   FCurrentRecBuf := FLastRecBuf;
 end;
@@ -1903,7 +1907,6 @@ end;
 
 procedure TCustomBufDataset.InternalDelete;
 var i         : Integer;
-    StartInd  : Integer;
     RemRec    : pointer;
     RemRecBookmrk : TBufBookmark;
     free_rec: Boolean;
@@ -1913,10 +1916,9 @@ begin
   // Remove the record from all active indexes
   FCurrentIndex.StoreCurrentRecIntoBookmark(@RemRecBookmrk);
   RemRec := FCurrentIndex.CurrentBuffer;
-  FIndexes[0].RemoveRecordFromIndex(RemRecBookmrk);
-  if FCurrentIndex=FIndexes[1] then StartInd := 1 else StartInd := 2;
-  for i := StartInd to FIndexesCount-1 do
-    findexes[i].RemoveRecordFromIndex(RemRecBookmrk);
+  for i := 0 to FIndexesCount-1 do
+    if (i<>1) or (FCurrentIndex=FIndexes[i]) then
+      FIndexes[i].RemoveRecordFromIndex(RemRecBookmrk);
 
   if not GetActiveRecordUpdateBuffer then
     begin
@@ -2153,10 +2155,11 @@ end;
 
 procedure TCustomBufDataset.InternalPost;
 
-Var CurrBuff     :  TRecordBuffer;
+Var ABuff        : TRecordBuffer;
     i            : integer;
     blobbuf      : tbufblobfield;
     NullMask     : pbyte;
+    ABookmark    : PBufBookmark;
 
 begin
   inherited InternalPost;
@@ -2164,43 +2167,48 @@ begin
    if assigned(FUpdateBlobBuffers[i]) and (FUpdateBlobBuffers[i]^.FieldNo>0) then
     begin
     blobbuf.BlobBuffer := FUpdateBlobBuffers[i];
-    CurrBuff := ActiveBuffer;
-    NullMask := pbyte(CurrBuff);
+    ABuff := ActiveBuffer;
+    NullMask := PByte(ABuff);
 
-    inc(CurrBuff,FFieldBufPositions[FUpdateBlobBuffers[i]^.FieldNo-1]);
-    Move(blobbuf, CurrBuff^, GetFieldSize(FieldDefs[FUpdateBlobBuffers[i]^.FieldNo-1]));
+    inc(ABuff,FFieldBufPositions[FUpdateBlobBuffers[i]^.FieldNo-1]);
+    Move(blobbuf, ABuff^, GetFieldSize(FieldDefs[FUpdateBlobBuffers[i]^.FieldNo-1]));
     unSetFieldIsNull(NullMask,FUpdateBlobBuffers[i]^.FieldNo-1);
     
     FUpdateBlobBuffers[i]^.FieldNo := -1;
     end;
 
-  if state = dsInsert then
+  if State = dsInsert then
     begin
-    if GetBookmarkFlag(ActiveBuffer) = bfEOF then
-      FIndexes[0].ScrollLast
-    else
-      // The active buffer is the newly created TDataset record,
-      // from which the bookmark is set to the record where the new record should be
-      // inserted
-      InternalSetToRecord(ActiveBuffer);
+    // The active buffer is the newly created TDataset record,
+    // from which the bookmark is set to the record where the new record should be
+    // inserted
+    ABookmark := PBufBookmark(ActiveBuffer + FRecordSize);
+    // Create the new record buffer
+    ABuff := IntAllocRecordBuffer;
 
-    with FIndexes[0] do
+    // Add new record to all active indexes
+    for i := 0 to FIndexesCount-1 do
+      if (i<>1) or (FIndexes[i]=FCurrentIndex) then
       begin
-      // Create the new record buffer
-      FCurrentIndex.InsertRecordBeforeCurrentRecord(IntAllocRecordBuffer);
-      ScrollBackward;
-      // Add the record to the other indexes
-      for i := 1 to FIndexesCount-1 do if ((i>1) or (FIndexes[i]=FCurrentIndex)) then
-        FIndexes[i].InsertRecordBeforeCurrentRecord(CurrentRecord);
+        if ABookmark^.BookmarkFlag = bfEOF then
+          // append (at end)
+          FIndexes[i].ScrollLast
+        else
+          // insert (before current record)
+          FIndexes[i].GotoBookmark(ABookmark);
+
+        FIndexes[i].InsertRecordBeforeCurrentRecord(ABuff);
+        // new inserted record becomes current record
+        FIndexes[i].ScrollBackward;
       end;
 
     // Link the newly created record buffer to the newly created TDataset record
-    with PBufBookmark(ActiveBuffer + FRecordSize)^ do
+    with ABookmark^ do
       begin
       FCurrentIndex.StoreCurrentRecIntoBookmark(@BookmarkData);
       BookmarkFlag := bfInserted;
       end;
-      
+
     inc(FBRecordCount);
     end
   else
@@ -2862,9 +2870,7 @@ begin
 
   if Active then
     begin
-    (FIndexes[FIndexesCount-1] as TDoubleLinkedBufIndex).FFirstRecBuf := pointer(IntAllocRecordBuffer);
-    (FIndexes[FIndexesCount-1] as TDoubleLinkedBufIndex).FLastRecBuf := (FIndexes[FIndexesCount-1] as TDoubleLinkedBufIndex).FFirstRecBuf;
-    (FCurrentIndex as TDoubleLinkedBufIndex).FCurrentRecBuf := (FIndexes[FIndexesCount-1] as TDoubleLinkedBufIndex).FLastRecBuf;
+    FIndexes[FIndexesCount-1].InitialiseSpareRecord(IntAllocRecordBuffer);
     BuildIndex(FIndexes[FIndexesCount-1]);
     end
   else if FIndexesCount>FMaxIndexesCount then

packages/fcl-db/tests/testdbbasics.pas

@@ -104,6 +104,7 @@ type
 
     procedure TestAddDblIndex;
     procedure TestIndexEditRecord;
+    procedure TestIndexAppendRecord;
   end;
 
 {$endif fpc}
@@ -1652,10 +1653,11 @@ begin
     first;
     ds.IndexName:='test';
     first;
-    LastValue:=FieldByName('name').AsString;
+    LastValue:='';
     while not eof do
       begin
       CheckTrue(AnsiCompareStr(LastValue,FieldByName('name').AsString)<=0);
+      LastValue:=FieldByName('name').AsString;
       Next;
       end;
     end;
@@ -1851,14 +1853,77 @@ begin
     FieldByName('F'+FieldTypeNames[AfieldType]).AsString := 'ZZZ';
     post;
     prior;
-    CheckTrue(AnsiCompareStr('ZZZ',FieldByName('F'+FieldTypeNames[AfieldType]).AsString)>=0);
+    CheckTrue(AnsiCompareStr('ZZZ',FieldByName('F'+FieldTypeNames[AfieldType]).AsString)>=0, 'Prior>');
     next;
     next;
-    CheckTrue(AnsiCompareStr('ZZZ',FieldByName('F'+FieldTypeNames[AfieldType]).AsString)<=0);
+    CheckTrue(AnsiCompareStr('ZZZ',FieldByName('F'+FieldTypeNames[AfieldType]).AsString)<=0, 'Next<');
     close;
     end;
 end;
 
+procedure TTestBufDatasetDBBasics.TestIndexAppendRecord;
+var i: integer;
+    LastValue: string;
+begin
+  with DBConnector.GetNDataset(true,0) as TCustomBufDataset do
+  begin
+    MaxIndexesCount:=4;
+    // add index to closed dataset with no data
+    AddIndex('testindex','NAME',[]);
+    IndexName:='testindex';
+    Open;
+    // empty dataset and other than default index (default_order) active
+    CheckTrue(BOF, 'No BOF when opening empty dataset');
+    CheckTrue(EOF, 'No EOF when opening empty dataset');
+
+    // append data at end
+    for i:=20 downto 0 do
+      AppendRecord([i, inttostr(i)]);
+    First;
+    // insert data at begining
+    for i:=21 to 22 do
+      InsertRecord([i, inttostr(i)]);
+
+    // ATM new records are not ordered as they are added ?
+    LastValue := '';
+    First;
+    for i:=22 downto 0 do
+    begin
+      CheckEquals(23-i, RecNo, 'testindex.RecNo:');
+      CheckEquals(inttostr(i), Fields[1].AsString, 'testindex.Fields[1].Value:');
+      //CheckTrue(AnsiCompareStr(LastValue,Fields[1].AsString) < 0, 'testindex.LastValue>CurrValue');
+      LastValue := Fields[1].AsString;
+      Next;
+    end;
+    CheckTrue(EOF, 'testindex.No EOF after last record');
+
+    // switch back to default index (unordered)
+    IndexName:='';
+    First;
+    for i:=22 downto 0 do
+    begin
+      CheckEquals(23-i, RecNo, 'index[0].RecNo:');
+      CheckEquals(i, Fields[0].AsInteger, 'index[0].Fields[0].Value:');
+      Next;
+    end;
+    CheckTrue(EOF, 'index[0].No EOF after last record');
+
+    // add index to opened dataset with data
+    AddIndex('testindex2','ID',[]);
+    IndexName:='testindex2';
+    First;
+    for i:=0 to 22 do
+    begin
+      CheckEquals(1+i, RecNo, 'index2.RecNo:');
+      CheckEquals(i, Fields[0].AsInteger, 'index2.Fields[0].Value:');
+      Next;
+    end;
+    CheckTrue(EOF, 'index2.No EOF after last record');
+
+    Close;
+  end;
+end;
+
 procedure TTestBufDatasetDBBasics.TestIndexFieldNames;
 var ds : TCustomBufDataset;
     AFieldType : TFieldType;
@@ -2361,7 +2426,8 @@ initialization
   RegisterTestDecorator(TDBBasicsTestSetup, TTestDBBasics);
   RegisterTestDecorator(TDBBasicsTestSetup, TTestCursorDBBasics);
 
-  if uppercase(dbconnectorname)='SQL' then
+  // The SQL connectors are descendents of bufdataset and therefore benefit from testing:
+  if (uppercase(dbconnectorname)='SQL') or (uppercase(dbconnectorname)='BUFDATASET') then
     begin
     RegisterTestDecorator(TDBBasicsTestSetup, TTestBufDatasetDBBasics);
     RegisterTestDecorator(TDBBasicsUniDirectionalTestSetup, TTestUniDirectionalDBBasics);