Hi,
I found this it fpc-pascal mailing list archive (I do not read this mailing list, so I do not catch this discussion) Because I think it is worth to continue, I move it into fpc-devel
/ ... I feel the definition
/>/ of SetFieldData/GetFieldData without a length/size parameter />/ and strings passed as pchar is causing all kind of problems:
/I agree/
/>/ 1) TStringField.SetAsString copies the string to a buffer />/ with size dsMaxStringSize so that datasets that don't figure />/ out the original length of the string can simply copy the />/ full Field.TDatasize (TBufDataset, TDbf, TMemDataset,...).
/Yes/
/>/ TWideStringField.SetAsString didn't which caused the crash />/ when the string is shorter than Field.Size.
/>/ TCustomSqliteDataset uses a StrNew(PChar(Buffer)); to get the />/ length of the string but fails sometimes (see 4).
/>/ 2) dsMaxStringSize isn't enforced for TStringField. Defining />/ a Field.size > dsMaxStringSize causes a crash in />/ TStringField.SetAsString. I haven't raised an issue on this, yet. /
Yes, it is also reported by fcl-db test suite, see: http://bugs.freepascal.org/view.php?id=19940
/ 3) some of the speed advantage of memory based datasets is
/>/ offset by moving full Field.Tdatasize bytes in both set and />/ get fielddata
/
/ 4) pascal strings can contain #0 characters in the string.
/>/ When converting to pchar part of these strings is lost. />/
/Very good point. See also comments in http://bugs.freepascal.org/view.php?id=19930
/
/>/ What I propose is:
/>/ 1 to create overloaded versions of SetFieldData and />/ GetFieldData that include a length parameter, to change />/ TStringField.SetAsString and TWideStringField.SetAsWideString />/ to use these versions
/Or also create procedure TField.SetData(Buffer: Pointer; DataSize : integer=0);
which will do some checks like existing TField.SetData(Buffer: Pointer; NativeFormat : Boolean)
and call new FDataSet.SetFieldData(Self, Buffer, NativeFormat, DataSize);
This new TDataSet.SetFieldData will by default call existing "old" SetFieldData:
procedure TDataSet.SetFieldData(Field: TField; Buffer: Pointer; NativeFormat: Boolean; DataSize:integer);
begin
SetFieldData(Field, Buffer, NativeFormat);
end;So backward compatibilty will be retained.
TDataSet descendants, which will want, they will override this new SetFieldData method and will take advantage of them. (in our case TBufDataSet will be adopted )
/
and to migrate the different datasets
/>/ to use these new versions. This way existing (user) code />/ using SetFieldData/GetFieldData will still work (and the user />/ still being responsible for buffer overruns...) and datasets />/ can correctly save and retrieve strings while improving performance. />/ 2 change the bufdataset internal storage for ftstring and />/ ftwidestring fields to include the string length so that />/ stored strings can be retrieved in full when containing #0 />/ characters and without having to copy systematically datasize />/ bytes.
/
Yes, so ftString will be stored like ftvarBinary, which has first two bytes length and followed by "binary string"
/ However this will introduce an incompatible binary
/>/ format for TFpcBinaryDatapacketReader. Having a second />/ FpcBinaryIdent would allow for both formats to co-habitate. />/
/>/ If need be the discussion can be moved to fpc-devel.
/Yes ;-)))/
/And another theme not related to this subjects are Autoincrement fields, which are ATM read-only for all access (BTW see also http://bugs.freepascal.org/view.php?id=17624 ) Here I have idea introduce at some level (TDataSet or TBufDataSet) virtual function (for example GetNextAutoIncValue), which will be called just after append...post (may be in InternalPost if State=dsInsert) And descendants like TSQLQuery will be able override this method and provide their own mechanism for retrieving auto-generated values from SQL databases. Like MySQL: select last_insert_rowid(), PostgreSQL: select lastval(), MSSQL: select @@identity, etc. For other solution (introduce new property RefreshSQL ) read: http://bugs.freepascal.org/view.php?id=16076 ATM auto-increment fields are practicaly useless.
Ofcourse this are only very raw ideas ...
TIA
-Laco.
