[fpc-devel] overloaded SetFieldData/GetFieldData

[LacaK]

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freepascal.org/pipermail/fpc-devel/attachments/20110819/41fd7c97/attachment.html>