View Issue Details

IDProjectCategoryView StatusLast Update
0015460FPCDatabasepublic2010-10-29 23:59
ReporterJosé Mejuto Assigned ToJoost van der Sluis  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindows 
Target Version2.4.2Fixed in Version2.6.0 
Summary0015460: Dataset SQLQuery crash on "IndexFieldNames"
DescriptionWhen using IndexFieldNames to alter the order of a read dataset the program will crash with a SIGSEGV.
Steps To ReproduceRun attached project (It is a Lazarus one as it is quite difficult to me to create a pure fpc project for databases). Adjust the connection to any existing database and maybe the SQL command to fetch a bunch of records and press any DBGrid column to order data by that field.

Only strictly necessary fields has been configured. The same problem appears with any database access, in the example I'm using Firebird.
Additional InformationThe problem seems to be in the TBufDataset.BuildIndex as the merge sort used does not reserve any additional space for the new list which from my point of view is mandatory in a merge sort. Also I had checked the referenced document (web page) and it also states that a temporal intermediate storage is needed, but in a non clear text, in fact in a paragraph he says something "[...] It avoids the need for the auxiliary space [...]" and a few lines below he says "[...] and also preparing an empty list L [...]" which denotes the need of an auxiliary space for the newly ordered list.

The prior-next releation seems to be broken after indexing, or if there are enought records just in the sort process. I was trying to fix it myself but the code is quite complex to understand and as I must recompile the compiler and lazarus lcl each time, and some problems with debugger converts it in a nightmare :( I think a more trained eyes will fix it in a "short" time.
TagsNo tags attached.
Fixed in Revision15235, 16257
FPCOldBugId
FPCTarget
Attached Files

Relationships

related to 0014979 closedJoost van der Sluis On TSQLQuery, when IndexFields are set before a Locate call, causes a SIGSEGV 

Activities

2010-01-25 11:58

 

hindexnames.pp (2,931 bytes)

2010-01-25 11:58

 

ebisfilltableparams.pp (1,742 bytes)

José Mejuto

2010-01-25 11:58

reporter   ~0033884

Using examples for sqldb the bug can be raised, adding a new example "hindexnames.pp" which apply a IndexFieldNames test over all the fields (The one over FFixedChar is commented out as it has been recently fixed). In the 3 performed tests the "Indexing ID" and "Indexing Birthday" works as expected, but the "Indexing Name" fails returning only 6 records instead 8. If you put more records in the base, like 1000 (ebisfilltableparams.pp), you end up with a SIGSEGV at some point.

Joost van der Sluis

2010-05-06 15:32

manager   ~0037323

Well.. it took some time to debug. But that only 6 records are shown is not a bug.
When you call dataset.first first, the current record has become the first record. When you change the order after that, the current records stays the same (ie: the record with the same content) but that could be the third record, not the first one. When you iterate through the dataset after that, the first two records are not shown.

I'll have to look at the sigsegv with more records.

Joost van der Sluis

2010-05-06 16:04

manager   ~0037324

Ok, there was a bug in TBufDataset.RecNo. It didn't use the current index to determine the record number. There was nothing wrong with the sort mechanism.

ps: You do know that every time you use TBufDataset.RecNo, it starts from the first record and iterates through all records until it find the current record and then returns the record number, don't you? ie: don't use TBufDataset.RecNo too much. Use bookmarks instead.

Joost van der Sluis

2010-05-06 16:07

manager   ~0037325

pps: And please be careful when you write a bug-report. It would have spared both of us a lot of time when you called it 'Getting the record number of a dataset after setting IndexFieldNames will crash with a SIGSEGV'. That was the line the crash happened on, after all.

José Mejuto

2010-08-31 01:02

reporter   ~0040666

The issue is not completly fixed, I think the "setrecno" should use the active index also in the same way as the getrecno. Attached a proposed fix (seems to work in my simple tests)

PS: Sorry about not being properly reported, but I had discovered it using lazarus DBGrid and the exception appears sometimes in the bufdataset, other times in the grid, and some of them without callback stack at all. I'm not using recno at all, its the grid which seems to use it.

2010-08-31 01:03

 

bufdataset_setrecno.patch (445 bytes)   
Index: bufdataset.pas
===================================================================
--- bufdataset.pas	(revision 15920)
+++ bufdataset.pas	(working copy)
@@ -2257,7 +2257,7 @@
     end;
   TmpRecBuffer := (FCurrentIndex as TDoubleLinkedBufIndex).FFirstRecBuf;
   for recnr := 1 to value-1 do
-    TmpRecBuffer := TmpRecBuffer^.next;
+    TmpRecBuffer := TmpRecBuffer[FCurrentIndex.IndNr].next;
   GotoBookmark(@TmpRecBuffer);
 end;
 
bufdataset_setrecno.patch (445 bytes)   

Issue History

Date Modified Username Field Change
2010-01-03 23:14 José Mejuto New Issue
2010-01-03 23:14 José Mejuto Status new => assigned
2010-01-03 23:14 José Mejuto Assigned To => Joost van der Sluis
2010-01-25 11:58 José Mejuto File Added: hindexnames.pp
2010-01-25 11:58 José Mejuto File Added: ebisfilltableparams.pp
2010-01-25 11:58 José Mejuto Note Added: 0033884
2010-05-06 15:32 Joost van der Sluis Note Added: 0037323
2010-05-06 16:04 Joost van der Sluis Fixed in Revision => 15235
2010-05-06 16:04 Joost van der Sluis Status assigned => resolved
2010-05-06 16:04 Joost van der Sluis Fixed in Version => 2.5.1
2010-05-06 16:04 Joost van der Sluis Resolution open => fixed
2010-05-06 16:04 Joost van der Sluis Note Added: 0037324
2010-05-06 16:04 Joost van der Sluis Target Version => 2.4.2
2010-05-06 16:07 Joost van der Sluis Status resolved => feedback
2010-05-06 16:07 Joost van der Sluis Resolution fixed => reopened
2010-05-06 16:07 Joost van der Sluis Note Added: 0037325
2010-05-06 16:08 Joost van der Sluis Status feedback => resolved
2010-05-06 16:08 Joost van der Sluis Resolution reopened => fixed
2010-05-06 16:17 Joost van der Sluis Relationship added related to 0014979
2010-08-31 01:02 José Mejuto Status resolved => feedback
2010-08-31 01:02 José Mejuto Resolution fixed => reopened
2010-08-31 01:02 José Mejuto Note Added: 0040666
2010-08-31 01:03 José Mejuto File Added: bufdataset_setrecno.patch
2010-10-29 21:20 Joost van der Sluis Fixed in Revision 15235 => 15235, 16257
2010-10-29 21:20 Joost van der Sluis Status feedback => resolved
2010-10-29 21:20 Joost van der Sluis Resolution reopened => fixed
2010-10-29 23:59 José Mejuto Status resolved => closed