View Issue Details

IDProjectCategoryView StatusLast Update
0020514FPCDatabasepublic2013-02-28 14:20
Reporterdave Liewald Assigned ToJoost van der Sluis  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Fixed in Version3.0.0 
Summary0020514: Adding index to Tbufdataset causes it to stop storing data
DescriptionMade a simple form containing just a tbufdataset component, added single string field to component, added button to the form to add 100 random strings to dataset ( inttostr(i) ), works fine. Then addindex to the dataset to sort the test field. Data Storage to db fails and no error is generated.
TagsNo tags attached.
Fixed in Revision21023
FPCOldBugId0
FPCTarget
Attached Files

Relationships

related to 0019631 assignedJoost van der Sluis Lazarus TBufDataset deletes all records when Refresh is applied 
related to 0023421 resolvedMichael Van Canneyt FPC Repeatedly duplicated entries when you add a new entry and use Index in BufDataset 
related to 0023465 closedMichael Van Canneyt FPC Adding data to indexed bufdataset does not sort it in correctly 
child of 0020648 resolvedJuha Manninen Lazarus Exception 'External: SIGSEGV' is raised after inserting a record in SQLQuery after sorting data. 

Activities

dave Liewald

2011-10-19 11:44

reporter   ~0053146

Small example prog

Tbufdataset1 dataset created on form creation


press adddata button 100 records stored, get data retrieves record 50

press addindes, adds simple index and sets bufdataset indexname to new index

press adddata 100 records posted , get data retrieve a blank record all records blank!

2011-10-19 11:50

 

tbufdataset error.zip (294,920 bytes)

Reinier Olislagers

2011-10-19 12:35

developer   ~0053152

Last edited: 2011-10-19 13:59

On Windows, Lazarus 0.9.31, SVN from 2011-10-19, FPC x86 2.5.1 SVN 32965M
- adddata, recordcount=100, get data retrieves result 50. Ok.
- press clear/addindex, ok.
- press adddata, recordcount=100, getdata retrives blank record.

Summary: results same as reporter.

dave Liewald

2011-11-02 10:34

reporter   ~0053737

Is there no Progress on this. It really is a showstopper for me and I would have thought it would affect all Tbufdataset and derivative functionality

dave Liewald

2011-11-24 12:03

reporter   ~0054424

Any Progress?

Ahmet Nuri

2012-01-14 18:43

reporter   ~0055750

if onyone need feed back for that bug.
You can read here
http://www.lazarus.freepascal.org/index.php/topic,15395.msg83112.html#msg83112

dave Liewald

2012-02-23 11:48

reporter   ~0057020

Still no progress? unfortunately I can't use a disk based solution as the accessing during lookup would be prohibitive

Marco van de Voort

2012-02-28 16:16

manager   ~0057149

I don't think anybody has worked on fundamental issues in bufdataset for over an year.

2012-03-04 13:33

 

bug20514.pp (1,071 bytes)

Marco van de Voort

2012-03-04 15:07

manager   ~0057269

The entire locate only seems to do only one record comparison, which doesn't sound right.

But debugging it is still a bit too hard for me.

dave Liewald

2012-03-22 15:54

reporter   ~0057923

I really hope someone cajn pick this up soon as it is a real showstopper for me!

LacaK

2012-04-04 09:55

developer   ~0058283

Last edited: 2012-04-05 13:45

You must do things in this order (AFAIU sources):
0. set MaxIndexesCount:=3
1. CreateDataSet
2. Add data
3. AddIndex (when dataset is active!)
4. Set active index (IndexName:=...)

If you add index when dataset is closed and then open it and add data then data are added to indexes in "insertion" order (see InternalPost and InsertRecordBeforeCurrentRecord) not index order (Why?).

I think, that I found some bugs, please see attached patch.
Because code is not trivial to understand I would be happy if somebody else will take a look at changes ... but my tests gives correct results.

2012-04-05 13:41

 

bufdataset1.diff (4,709 bytes)   
--- bufdataset.pas.ori	Thu Apr 05 12:15:10 2012
+++ bufdataset.pas	Thu Apr 05 13:35:34 2012
@@ -951,14 +951,16 @@ begin
       PCurRecLinkItem[DblLinkIndex.IndNr].next := PCurRecLinkItem[0].next;
       PCurRecLinkItem[DblLinkIndex.IndNr].prior := PCurRecLinkItem[0].prior;
       end;
-    end;
 
-// Set FirstRecBuf and FCurrentRecBuf
-  DblLinkIndex.FFirstRecBuf:=Index0.FFirstRecBuf;
-  (FCurrentIndex as TDoubleLinkedBufIndex).FCurrentRecBuf:=DblLinkIndex.FFirstRecBuf;
-// Link in the FLastRecBuf that belongs to this index
-  PCurRecLinkItem[DblLinkIndex.IndNr].next:=DblLinkIndex.FLastRecBuf;
-  DblLinkIndex.FLastRecBuf[DblLinkIndex.IndNr].prior:=PCurRecLinkItem;
+    // Set FirstRecBuf and FCurrentRecBuf
+    DblLinkIndex.FFirstRecBuf:=Index0.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;
+    end
+  else
+    Exit;
 
 // Mergesort. Used the algorithm as described here by Simon Tatham
 // http://www.chiark.greenend.org.uk/~sgtatham/algorithms/listsort.html
@@ -975,7 +977,7 @@ begin
 // of as we finish dealing with them.
 
   p := DblLinkIndex.FFirstRecBuf;
-  DblLinkIndex.ffirstRecBuf := nil;
+  DblLinkIndex.FFirstRecBuf := nil;
   q := p;
   MergeAmount := 0;
 
@@ -1079,7 +1081,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 +1205,7 @@ procedure TCustomBufDataset.InternalLast
 begin
   FetchAll;
   with FCurrentIndex do
-  SetToLastRecord;
+    SetToLastRecord;
 end;
 
 function TDoubleLinkedBufIndex.GetCurrentRecord: TRecordBuffer;
@@ -1362,6 +1364,7 @@ procedure TDoubleLinkedBufIndex.Initiali
 begin
   FFirstRecBuf := pointer(ASpareRecord);
   FLastRecBuf := FFirstRecBuf;
+  FLastRecBuf[IndNr].prior:=nil;
   FLastRecBuf[IndNr].next:=FLastRecBuf;
   FCurrentRecBuf := FLastRecBuf;
 end;
@@ -1916,7 +1919,7 @@ begin
   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);
+    FIndexes[i].RemoveRecordFromIndex(RemRecBookmrk);
 
   if not GetActiveRecordUpdateBuffer then
     begin
@@ -2174,25 +2177,23 @@ begin
     FUpdateBlobBuffers[i]^.FieldNo := -1;
     end;
 
-  if state = dsInsert then
+  if State = dsInsert then
     begin
     if GetBookmarkFlag(ActiveBuffer) = bfEOF then
-      FIndexes[0].ScrollLast
+      FCurrentIndex.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);
 
-    with FIndexes[0] do
-      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);
-      end;
+    // Create the new record buffer
+    FCurrentIndex.InsertRecordBeforeCurrentRecord(IntAllocRecordBuffer);
+    FCurrentIndex.ScrollBackward;
+    // Add the record to the other indexes
+    for i := 0 to FIndexesCount-1 do
+      if (FIndexes[i] <> FCurrentIndex) then
+        FIndexes[i].InsertRecordBeforeCurrentRecord(FCurrentIndex.CurrentRecord);
 
     // Link the newly created record buffer to the newly created TDataset record
     with PBufBookmark(ActiveBuffer + FRecordSize)^ do
@@ -2845,9 +2846,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
bufdataset1.diff (4,709 bytes)   

Reinier Olislagers

2012-04-05 17:02

developer   ~0058326

Ran dbtestframework with bufdataset on FPC Win x86. No differences before and after patch.... so probably the test framework does not yet cover the problems in the bug report...

2012-04-10 08:02

 

bufdataset2.diff (6,396 bytes)   
--- bufdataset.pas.ori	Thu Apr 05 12:15:10 2012
+++ bufdataset.pas	Tue Apr 10 07:27:04 2012
@@ -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.Initiali
 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;
+
+    // Add new record to all active indexes
+    for i := 0 to FIndexesCount-1 do
+      if (i<>1) or (FIndexes[i]=FCurrentIndex) then
+      begin
+        if ABookmark^.BookmarkFlag = bfEOF then
+          // append (at end)
+          FIndexes[i].ScrollLast
+        else
+          // insert (before current record)
+          FIndexes[i].GotoBookmark(ABookmark);
 
-    with FIndexes[0] do
-      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);
+        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
@@ -2845,9 +2853,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
bufdataset2.diff (6,396 bytes)   

2012-04-10 08:02

 

testdbbasics2.diff (3,259 bytes)   
--- testdbbasics.pas.ori	Tue Apr 10 07:04:40 2012
+++ testdbbasics.pas	Tue Apr 10 08:00:12 2012
@@ -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,12 +1853,75 @@ 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;
testdbbasics2.diff (3,259 bytes)   

LacaK

2012-04-10 08:05

developer   ~0058445

Improved fix + test for dbtestframework (with unpatched version of bufdataset.pas will fail)

Reinier Olislagers

2012-04-10 14:21

developer   ~0058469

Thanks for the update and test case, Lacak!

Needed additional patch (attached) for testdbbasics.pas to get bufdataset tests working.

FPC r20780, win x86 and Win x86=>x64 cross compiler without the fix indeed give
<test name="TTestBufDatasetDBBasics.TestIndexAppendRecord">
<failure ExceptionClassName="EAssertionFailedError">
<message>No BOF when opening empty dataset expected: <True> but was: <False></message>
</failure>

<NumberOfRunnedTests>86</NumberOfRunnedTests>
<NumberOfErrors>17</NumberOfErrors>
<NumberOfFailures>33</NumberOfFailures>

Patched, that test succeeds on x86 and x64 and reduced the number of errors and failures:
<NumberOfRunnedTests>86</NumberOfRunnedTests>
<NumberOfErrors>15</NumberOfErrors>
<NumberOfFailures>26</NumberOfFailures>

2012-04-10 14:21

 

testbufdataset.diff (644 bytes)   
Index: testdbbasics.pas
===================================================================
--- testdbbasics.pas	(revision 20780)
+++ testdbbasics.pas	(working copy)
@@ -2363,9 +2363,13 @@
 
   if uppercase(dbconnectorname)='SQL' then
     begin
-    RegisterTestDecorator(TDBBasicsTestSetup, TTestBufDatasetDBBasics);
     RegisterTestDecorator(TDBBasicsUniDirectionalTestSetup, TTestUniDirectionalDBBasics);
     end;
+
+  if uppercase(dbconnectorname)='BUFDATASET' then
+    begin   
+    RegisterTestDecorator(TDBBasicsTestSetup, TTestBufDatasetDBBasics);    
+    end;
 {$else fpc}
   RegisterTest(TTestDBBasics.Suite);
 {$endif fpc}
testbufdataset.diff (644 bytes)   

Ludo Brands

2012-04-10 17:12

developer   ~0058479

Appending and inserting a record with an active index is still not working correctly. The new record is inserted at the current location or appended at the end and stays there instead of the index entry being moved to the correct, sorted location. As a result, creating an index works on existing data but the index breaks when adding new data. Ex: One field dataset with ascending index on field. Data is 5,7,9. Append 2,10. Resulting dataset: 5,7,9,2,10 instead of 2,5,7,9,10.

LacaK

2012-04-11 07:36

developer   ~0058490

Last edited: 2012-04-11 09:43

@Reinier: I do not think, that we must remove test from "sql" and move in "bufdataset". When I run tests for any sqlDB connection then also bufdataset is tested, which is good (TSQLQuery is descendant of TCustomBufDataset). So IMHO leave it as is good. Of course we can ADD "bufdataset", but also fixing "unidirectional" and OTHER things in bufdatasettoolsunit is required ... so please create another bug report for that if you want.

@Ludo: Yes I am aware of that. It is not only case of Append/Insert but also Edit. When existing record is changed then no re-sorting occurs (or record is not inserted or moved to proper position). But this is more another "missing feature" than this bug report (as we can see in sources this is not implemented at all). This patch fixes "ONLY" bug described in this bug report. Later (in next step) we can create another bug report (if somebody is interested in ;-)).

Ludo Brands

2012-04-11 08:56

developer   ~0058492

@LacoK2: from the attached test file you know what the original reporter tries to do: create index and add data. Yes, the data were not stored and the patch now stores them. But they are stored in the wrong place (ie index is not updated). The symptoms are gone but the patient is dead.
This being said, I admire your perseverance in trying to fix up an overly complex tbufdataset

LacaK

2012-04-11 09:42

developer   ~0058494

@Ludo: I agree with you, believe me! My only request is let we do changes in steps. To be careful.

Reinier Olislagers

2012-04-11 11:06

developer   ~0058495

Last edited: 2012-04-11 11:09

@Lacak2: agreed that testing also for SQL is beneficial.
I'm adding testbufdataset2.pas (replacement for testbufdataset.pas) here though: testing bufdataset alone will eliminate added complexity/influences from the sql connectors; the bug report was about bufdataset, not an sql connector. This way, the test case you wrote can be used more accurately.
If a dev however still wants me to open up a separate report for this, I'd be willing to do so.

Also, thanks for your determination in trying to fix this, as Ludo said.

2012-04-11 11:07

 

testbufdataset2.diff (724 bytes)   
Index: testdbbasics.pas
===================================================================
--- testdbbasics.pas	(revision 20781)
+++ testdbbasics.pas	(working copy)
@@ -2361,7 +2361,8 @@
   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);
testbufdataset2.diff (724 bytes)   

LacaK

2012-04-12 12:39

developer   ~0058529

So IMO there is now agreement, that commiting *2.diff is step in right direction (from worse to better).
So they can be applied as first step.

Marco van de Voort

2012-04-20 17:20

manager   ~0058805

The code in internaldelete() looks dodgy. Old code only skips index 1 if it is the currentindex, the new code always skips it.

LacaK

2012-04-23 07:00

developer   ~0058911

AFAIU, old code skips index 1 if it is NOT CurrentIndex
new code removes record from all indexes other than 1 OR from index 1 if it is CurrentIndex.

Purpose of code is "remove from all indexes other than 1" + "remove from index 1 only if it as CurrentIndex (so is active)"

So I think, that all is as expected ;-)

Marco van de Voort

2012-04-24 19:48

manager   ~0058949

Committed, thanks.

dave Liewald

2012-07-03 14:48

reporter   ~0060832

As there are problems with getting Lazarus to run in conjunction with 2.7.1 at the moment would it be possible to backport this fix into 2.6.1?

LacaK

2012-07-04 07:13

developer   ~0060841

AFAIK this patch is already part of 2.6 fixies, so it will be also part of incoming 2.6.2 release.

Marco van de Voort

2012-07-04 10:17

manager   ~0060843

See Lacak2's comment.

Issue History

Date Modified Username Field Change
2011-10-19 10:35 dave Liewald New Issue
2011-10-19 10:35 dave Liewald Widgetset => GTK 2
2011-10-19 10:43 Vincent Snijders Project Lazarus => FPC
2011-10-19 11:44 dave Liewald Note Added: 0053146
2011-10-19 11:50 dave Liewald File Added: tbufdataset error.zip
2011-10-19 12:35 Reinier Olislagers Note Added: 0053152
2011-10-19 13:59 Reinier Olislagers Note Edited: 0053152
2011-10-20 13:39 Jonas Maebe FPCOldBugId => 0
2011-10-20 13:39 Jonas Maebe Category Database => Database Components
2011-10-20 13:39 Jonas Maebe Product Version 0.9.30.1 (SVN) =>
2011-11-02 10:34 dave Liewald Note Added: 0053737
2011-11-02 18:51 Zeljan Rikalo Relationship added related to 0019631
2011-11-08 10:58 Zeljan Rikalo Relationship added child of 0020648
2011-11-24 12:03 dave Liewald Note Added: 0054424
2012-01-14 18:43 Ahmet Nuri Note Added: 0055750
2012-01-14 21:30 Florian Status new => assigned
2012-01-14 21:30 Florian Assigned To => Joost van der Sluis
2012-02-23 11:48 dave Liewald Note Added: 0057020
2012-02-28 16:16 Marco van de Voort Note Added: 0057149
2012-03-04 13:33 Marco van de Voort File Added: bug20514.pp
2012-03-04 15:07 Marco van de Voort Note Added: 0057269
2012-03-22 15:54 dave Liewald Note Added: 0057923
2012-04-04 09:55 LacaK Note Added: 0058283
2012-04-04 10:06 LacaK Note Edited: 0058283
2012-04-05 13:41 LacaK File Added: bufdataset1.diff
2012-04-05 13:45 LacaK Note Edited: 0058283
2012-04-05 17:02 Reinier Olislagers Note Added: 0058326
2012-04-10 08:02 LacaK File Added: bufdataset2.diff
2012-04-10 08:02 LacaK File Added: testdbbasics2.diff
2012-04-10 08:05 LacaK Note Added: 0058445
2012-04-10 14:21 Reinier Olislagers Note Added: 0058469
2012-04-10 14:21 Reinier Olislagers File Added: testbufdataset.diff
2012-04-10 17:12 Ludo Brands Note Added: 0058479
2012-04-11 07:36 LacaK Note Added: 0058490
2012-04-11 08:37 LacaK Note Edited: 0058490
2012-04-11 08:56 Ludo Brands Note Added: 0058492
2012-04-11 09:42 LacaK Note Added: 0058494
2012-04-11 09:43 LacaK Note Edited: 0058490
2012-04-11 11:06 Reinier Olislagers Note Added: 0058495
2012-04-11 11:07 Reinier Olislagers File Added: testbufdataset2.diff
2012-04-11 11:09 Reinier Olislagers Note Edited: 0058495
2012-04-12 12:39 LacaK Note Added: 0058529
2012-04-20 17:20 Marco van de Voort Note Added: 0058805
2012-04-23 07:00 LacaK Note Added: 0058911
2012-04-24 19:48 Marco van de Voort Fixed in Revision => 21023
2012-04-24 19:48 Marco van de Voort Status assigned => resolved
2012-04-24 19:48 Marco van de Voort Fixed in Version => 2.7.1
2012-04-24 19:48 Marco van de Voort Resolution open => fixed
2012-04-24 19:48 Marco van de Voort Note Added: 0058949
2012-07-03 14:48 dave Liewald Status resolved => feedback
2012-07-03 14:48 dave Liewald Resolution fixed => reopened
2012-07-03 14:48 dave Liewald Note Added: 0060832
2012-07-04 07:13 LacaK Note Added: 0060841
2012-07-04 10:17 Marco van de Voort Status feedback => resolved
2012-07-04 10:17 Marco van de Voort Resolution reopened => fixed
2012-07-04 10:17 Marco van de Voort Note Added: 0060843
2012-12-07 10:51 Reinier Olislagers Relationship added related to 0023421
2012-12-11 13:10 Reinier Olislagers Relationship added related to 0023465