zMemtable
Re: zMemtable
Michal,
Don't use it. I don't know if the size is reported incorrectly or we are typecasting the wrong pointer in GetData. I'll leave this fix to Michael, he has more knowledge about this than I do.
Don't use it. I don't know if the size is reported incorrectly or we are typecasting the wrong pointer in GetData. I'll leave this fix to Michael, he has more knowledge about this than I do.
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
Hi aehimself,
The 'kjteng' version seems to be better at ftDate and works with Lazarus 2.0.12-Win64. But it also has a problem with inserting zeros.
You could take a peek, maybe you can correct it. I send.
Michał
The 'kjteng' version seems to be better at ftDate and works with Lazarus 2.0.12-Win64. But it also has a problem with inserting zeros.
You could take a peek, maybe you can correct it. I send.
Michał
You do not have the required permissions to view the files attached to this post.
Re: zMemtable
Michal,
The error which is causing the zero date is not in the SaveToStream method, but misaligned data size reported and actual data size. The solution resides in TZDateField & TZTimeField or TZAbstractRODataSet. We actually found a bug, I just don't know which way I should fix it - that's why I am waiting for Michael.
Does your comment mean that the current in-place implementation does not work on Lazarus?
Once we consider it stable-enough and I get rid of the compiler directive I can modify the test suite to have stream testing.
The error which is causing the zero date is not in the SaveToStream method, but misaligned data size reported and actual data size. The solution resides in TZDateField & TZTimeField or TZAbstractRODataSet. We actually found a bug, I just don't know which way I should fix it - that's why I am waiting for Michael.
I asked several times for feedback on Lazarus and as there was none I considered it fully functional.
Does your comment mean that the current in-place implementation does not work on Lazarus?
Once we consider it stable-enough and I get rid of the compiler directive I can modify the test suite to have stream testing.
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
Hi aehimself,
Therefore, I asked for a correction as an alternative solution.
ADD: Your version also has a problem with WIDEMEMO in Lazarus.
Michał
It works, but with a (critical) Date error which is not present in the uploaded 'kjteng' version (it only has a problem with zeroing empty numbers).
Therefore, I asked for a correction as an alternative solution.
ADD: Your version also has a problem with WIDEMEMO in Lazarus.
Michał
-
- Platinum Boarder
- Posts: 1939
- Joined: 17.01.2011, 14:17
Re: zMemtable
Regarding TZDateField.GetDataSize: I assume for Zeos itself it doesn't matter much. One reason for Michaels implementation of TZ*Fields is that they don't use the Delphi record buffer. They use the RowAccessor directly. So these Fields don't need to care about GetDataSize that much, I think.aehimself wrote: ↑10.11.2021, 20:31 It's a .GetData bug, probably like at Blobs. Use ftDateTime instead, it doesn't seem to be affected. I'll look into it but I won't touch anything else than MemTable until Michael confirms if this behavior is normal or not.
Edit: Strange but no, it's not the same. ftDate, ftTime and ftDateTime are all coded down correctly, but it does not work.
ZAbstractRODataSet.pas : 2859 (TZAbstractRODataset.GetFieldData)DT correctly receives it's data but the assignment above does absolutely nothing to Buffer, it stays [0, 0, 0, 0]. The reason is, we are assigning a TDateTime (Double, which is 8 bytes) to a 4 byte buffer. 4 bytes, that is coming from TDateField.GetDataSize, which returns SizeOf(Integer).Code: Select all
else PDateTime(Buffer)^ := DT
This is actual memory corruption, so if Zeos is using DataSize and GetData internally this must be looked at.
Am am summoning Michael here. Are we missing an override in TZDateField...?
Edit-edit: Can be. Overriding TZDateField's (and TZTimeField's) GetDataSize to return this:
does fix the issue. On the other hand I'm not sure how the internal design of TZFields work, so - as I mentioned - I'm not touching thisCode: Select all
function TZDateField.GetDataSize: Integer; begin Result := SizeOf(TDateTime); end;
I think we should test your correction. Using our test suites, we have a good way to test these changes.
Re: zMemtable
That is good news, no memory corruption in Zeos I suppose.marsupilami wrote: ↑11.11.2021, 21:52Regarding TZDateField.GetDataSize: I assume for Zeos itself it doesn't matter much. One reason for Michaels implementation of TZ*Fields is that they don't use the Delphi record buffer. They use the RowAccessor directly. So these Fields don't need to care about GetDataSize that much, I think.
On the other hand Zeos 8 is happy run on D7, which suggests we do care about backwards compatibility. In that case - I think - we should update all related fields of properties we touched.
I mean, just because Zeos doesn't need it, something else might does.
Also, the issue is still present if you set DisableZFields to false on the dataset, which is bad :) DataSize still comes from TDateField, buffer still gets filled by TZAbstractRODataSet.
Sure thing, I can issue a pull request with it. I'm just feeling extra unsafe on grounds I never wondered upon. I'm trying not to break functionality knowingly.marsupilami wrote: ↑11.11.2021, 21:52I think we should test your correction. Using our test suites, we have a good way to test these changes. :)
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
My GitHub fork contains the commit which reports a larger data size for TZDateField and TZTimeField as Jan suggested. A pull request is also open to merge these changes back so Jenkins can shout at me if this breaks things.
In the mean time, SaveToStream in my fork dumps time fields corretly, so Michal's test case is successful.
I advise to wait for the tests before using it, though.
@ Michal,
Can you please provide some details on the FPC issues? Without being able to test it myself, "issue with widememo" and "critical date" doesn't mean much without symptoms / stack traces.
In the mean time, SaveToStream in my fork dumps time fields corretly, so Michal's test case is successful.
I advise to wait for the tests before using it, though.
@ Michal,
Can you please provide some details on the FPC issues? Without being able to test it myself, "issue with widememo" and "critical date" doesn't mean much without symptoms / stack traces.
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
Hi aehimself,
Now in D10.3.3-Win32 it works in my testing.
In Lazarus 2.0.12-Win64 crashes when trying to SaveToFile for any table with non-empty MEMO field
This is best seen by downloading:
https://sourceforge.net/projects/lazaru ... %202.0.12/
or other:
https://sourceforge.net/projects/lazarus/files/
you run the installer, and after 5-10 minutes you have Lazarus running
And then:
viewtopic.php?f=3&t=44184
ADD: Also works with Delphi 2007
Michał
Now in D10.3.3-Win32 it works in my testing.
In Lazarus 2.0.12-Win64 crashes when trying to SaveToFile for any table with non-empty MEMO field
This is best seen by downloading:
https://sourceforge.net/projects/lazaru ... %202.0.12/
or other:
https://sourceforge.net/projects/lazarus/files/
you run the installer, and after 5-10 minutes you have Lazarus running
And then:
viewtopic.php?f=3&t=44184
ADD: Also works with Delphi 2007
Michał
Re: zMemtable
Also works with Delphi 2007 but sometimes with LoadFromStream the error 'Invalid floating point operation'
pops up and the stop is on:
ZSysUtils.pas:
Michał
pops up and the stop is on:
ZSysUtils.pas:
Code: Select all
procedure DecodeDateTimeToDate(const Value: TDateTime; var Date: TZDate);
begin
DecodeDate(Value, Date.Year, Date.Month, Date.Day);
Date.IsNegative := False;
end;
Re: zMemtable
Okay, so the thing is, it doesn't matter if I override TZDateField's GetDataSize, my Delphi 7 installation creates a normal TDateField instead.
We need the WITH_GETFIELDCLASS_TFIELDDEF_OVERLOAD directive, which is defined in Zeos.inc under:
{$IF CompilerVersion >= 20} //Delphi 2009+
So it seems everything below Delphi 2009 will still get 4 bytes as DataSize and copy 8 bytes via GetData.
This seems wrong to me. Shouldn't .GetData actually check if we created a TZField, otherwise just use the PInteger version...?!
I bet Lazarus will be the same, therefore the Date issue Michal reported.
We need the WITH_GETFIELDCLASS_TFIELDDEF_OVERLOAD directive, which is defined in Zeos.inc under:
{$IF CompilerVersion >= 20} //Delphi 2009+
So it seems everything below Delphi 2009 will still get 4 bytes as DataSize and copy 8 bytes via GetData.
This seems wrong to me. Shouldn't .GetData actually check if we created a TZField, otherwise just use the PInteger version...?!
I bet Lazarus will be the same, therefore the Date issue Michal reported.
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
All,
My fork now contains a commit which half-implements kjteng's idea. Now Memo and WideMemo fields are read / written as String, only blobs are saved as streams, the rest is saved with GetData / SetData with an in-place fix for ftDate and ftTime fields.
Please check. Test cases succeed on D7 and D10.4.2.
My fork now contains a commit which half-implements kjteng's idea. Now Memo and WideMemo fields are read / written as String, only blobs are saved as streams, the rest is saved with GetData / SetData with an in-place fix for ftDate and ftTime fields.
Please check. Test cases succeed on D7 and D10.4.2.
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
Hi aehimself,
For D10.3.3-Win32 and LoadFromFile still 'Out of memory' sometimes 'Stream read error'.
Michał
For D10.3.3-Win32 and LoadFromFile still 'Out of memory' sometimes 'Stream read error'.
Michał
Re: zMemtable
As I'm getting close to none test cases, I created one for myself.
The code above executed successfully on D7 and D10.4.2 Win32 and Win64. Blobs values are not verified as I couldn't really care to write the CompareMem verification for .AsBytes :)
The source of the newest Stream read error was a damn variable I forgot to change, I don't know how it did not show up in the first two tests.
My fork contains the fix, a pull request was also issued.
Please check and confirm. Thanks.
Code: Select all
procedure TForm1.Button3Click(Sender: TObject);
Function FieldVal(Const inFieldName: String): Variant;
Begin
If inFieldName = 'String' Then Result := 'Testing, Testing'
Else
If inFieldName = 'SmallInt' Then Result := 32767
Else
If inFieldName = 'Integer' Then Result := 1000000
Else
If inFieldName = 'Word' Then Result := 65535
Else
If inFieldName = 'Boolean' Then Result := True
Else
If inFieldName = 'Float' Then Result := 3.14
Else
If inFieldName = 'Date' Then Result := '2011/11/11'
Else
If inFieldName = 'Time' Then Result := '6:15'
Else
If inFieldName = 'DateTime' Then Result := '2011/11/11 6:15'
Else
If inFieldName = 'Blob' Then Result := 1986 // VarArrayOf([19, 86, 10, 15])
Else
If inFieldName = 'Memo' Then Result := 'Hello, world!'
Else
If inFieldName = 'WideString' Then Result := 'The quick brown fox jumps over the lazy dog'
Else
If inFieldName = 'LargeInt' Then Result := 9223372036854775807
Else
If inFieldName = 'WideMemo' Then Result := 'WideMemo Test Data'
Else
If inFieldName = 'LongWord' Then Result := 4294967295
Else
If inFieldName = 'ShortInt' Then Result := 127
Else
If inFieldName = 'Byte' Then Result := 255
Else
If inFieldName = 'Single' Then Result := 3.14;
End;
Var
a: Integer;
begin
ClearAll;
ZMemTable1.FieldDefs.Add('String', ftString, 100);
ZMemTable1.FieldDefs.Add('SmallInt', ftSmallInt);
ZMemTable1.FieldDefs.Add('Integer', ftInteger);
ZMemTable1.FieldDefs.Add('Word', ftWord);
ZMemTable1.FieldDefs.Add('Boolean', ftBoolean);
ZMemTable1.FieldDefs.Add('Float', ftFloat);
ZMemTable1.FieldDefs.Add('Date', ftDate);
ZMemTable1.FieldDefs.Add('Time', ftTime);
ZMemTable1.FieldDefs.Add('DateTime', ftDateTime);
ZMemTable1.FieldDefs.Add('Blob', ftBlob);
ZMemTable1.FieldDefs.Add('Memo', ftMemo);
ZMemTable1.FieldDefs.Add('WideString', ftWideString, 100);
// ZMemTable1.FieldDefs.Add('LargeInt', ftLargeInt);
// ZMemTable1.FieldDefs.Add('WideMemo', ftWideMemo);
// ZMemTable1.FieldDefs.Add('LongWord', ftLongWord);
// ZMemTable1.FieldDefs.Add('ShortInt', ftShortInt);
// ZMemTable1.FieldDefs.Add('Byte', ftByte);
// ZMemTable1.FieldDefs.Add('Single', ftSingle);
ZMemTable1.Open;
ZMemTable1.Append;
ZMemTable1.Post;
ZMemTable1.Append;
For a := 0 To ZMemTable1.FieldCount - 1 Do
ZMemTable1.Fields[a].Value := FieldVal(ZMemTable1.Fields[a].FieldName);
ZMemTable1.Post;
SaveAndReload;
ZMemTable1.First;
For a := 0 To ZMemTable1.FieldCount - 1 Do
If Not ZMemTable1.Fields[a].IsNull Then Raise Exception.Create('Field ' + ZMemTable1.Fields[a].FieldName + ' is not null, however it should be!');
ZMemTable1.Next;
For a := 0 To ZMemTable1.FieldCount - 1 Do
Try
If ZMemTable1.Fields[a].FieldName <> 'Blob' Then If ZMemTable1.Fields[a].Value <> FieldVal(ZMemTable1.Fields[a].FieldName) Then Raise Exception.Create('Field ' + ZMemTable1.Fields[a].FieldName + ' value error');
Except
On E:Exception Do Raise Exception.Create('Field ' + ZMemTable1.Fields[a].FieldName + ' value error: ' + E.Message);
End;
end;
The source of the newest Stream read error was a damn variable I forgot to change, I don't know how it did not show up in the first two tests.
My fork contains the fix, a pull request was also issued.
Please check and confirm. Thanks.
Delphi 12.1, Zeos 8 from latest GIT snapshot
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Using:
- MySQL server 8.0.18; libmariadb.dll 3.3.8
- Oracle server 11.2.0, 12.1.0, 19.0.0; oci.dll 21.13
- MSSQL 2012, 2019; sybdb.dll FreeTDS_2435
- SQLite 3.45.2
Re: zMemtable
Hi aehimself,
With your patch from an hour ago(for Zeos8_trunk7715):
L606: - AStream.Read(buf, Length(buf));
+ AStream.Read(buf, fsize);
My test on FirebirdSQL database tables passed for:
- Delphi 2007
- Delphi XE2-Win32/Win64
- Delphi 10.3.3-Win32/Win64
- Delphi 11sp1-Win32/Win64
- Lazarus 2.0.8-Win32
- Lazarus 2.0.12-Win64
- Lazarus 2.2.0RC2-Win64
Uff
Michał
With your patch from an hour ago(for Zeos8_trunk7715):
L606: - AStream.Read(buf, Length(buf));
+ AStream.Read(buf, fsize);
My test on FirebirdSQL database tables passed for:
- Delphi 2007
- Delphi XE2-Win32/Win64
- Delphi 10.3.3-Win32/Win64
- Delphi 11sp1-Win32/Win64
- Lazarus 2.0.8-Win32
- Lazarus 2.0.12-Win64
- Lazarus 2.2.0RC2-Win64
Uff
Michał
Last edited by miab3 on 14.11.2021, 08:34, edited 1 time in total.
Re: zMemtable
Hi, aehimself:
I have just downloaded and tested on Lazarus 2.0.12 and have two observations bellow :
1. Line 722 (Savetostream), if we include ftstring in the first case (see below), the stream/file size will reduced significantly
Case field.DataType Of
ftString, ftMemo {$IFDEF WITH_WIDEMEMO}, ftWideMemo{$ENDIF}: WriteString(field.AsString); // <--add ftString
....
(LoadFromStream method also need to be changed accordingly)
2. Line 544, it will be safer to reset the position of Astream before reading the fieldcount (this is because the AStream passed to the method might not be in the 0 position), as follows:
try
AStream.Position := 0; // <- set stream cursor position to 0
// Recreate FieldDefs
len := ReadInt;
Hope you will consider and incorporate the above suggestion.
I have just downloaded and tested on Lazarus 2.0.12 and have two observations bellow :
1. Line 722 (Savetostream), if we include ftstring in the first case (see below), the stream/file size will reduced significantly
Case field.DataType Of
ftString, ftMemo {$IFDEF WITH_WIDEMEMO}, ftWideMemo{$ENDIF}: WriteString(field.AsString); // <--add ftString
....
(LoadFromStream method also need to be changed accordingly)
2. Line 544, it will be safer to reset the position of Astream before reading the fieldcount (this is because the AStream passed to the method might not be in the 0 position), as follows:
try
AStream.Position := 0; // <- set stream cursor position to 0
// Recreate FieldDefs
len := ReadInt;
Hope you will consider and incorporate the above suggestion.