View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0015460||FPC||Database||public||2010-01-03 23:14||2010-10-29 23:59|
|Reporter||José Mejuto||Assigned To||Joost van der Sluis|
|Target Version||2.4.2||Fixed in Version||2.6.0|
|Summary||0015460: Dataset SQLQuery crash on "IndexFieldNames"|
|Description||When using IndexFieldNames to alter the order of a read dataset the program will crash with a SIGSEGV.|
|Steps To Reproduce||Run 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 Information||The 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.
|Tags||No tags attached.|
|Fixed in Revision||15235, 16257|
hindexnames.pp (2,931 bytes)
ebisfilltableparams.pp (1,742 bytes)
||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.|
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.
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.
||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.|
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.
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)
|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|