View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0033010||FPC||Packages||public||2018-01-12 11:53||2018-01-14 10:29|
|Reporter||Ondrej Pokorny||Assigned To||Michael Van Canneyt|
|Product Version||3.1.1||Product Build|
|Target Version||3.2.0||Fixed in Version||3.1.1|
|Summary||0033010: [fcl-db] MSSQL: An SQL statement is always prepared even if it's been prepared already|
|Description||In MSSQL, when the statement is prepared, the FPrepared flag is not set to true so that it has to be prepared all over again which is useless.|
|Steps To Reproduce||1.) Apply the attached patch (but comment out the "FPrepared := True;" line).|
2.) Run this program:
db, mssqlconn, sqldb, sysutils;
xSQL := TMSSQLConnection.Create(nil);
xSQL.HostName := 'your-server';
xSQL.DatabaseName := 'your-database';
xSQL.Connected := True;
Q := TSQLQuery.Create(xSQL);
T := TSQLTransaction.Create(xSQL);
T.DataBase := xSQL;
Q.DataBase := xSQL;
Q.Transaction := T;
Writeln(sLineBreak, 'Statement start here', sLineBreak);
Q.SQL.Text := 'SELECT mydate from mytest WHERE CAST(mydate as DATE)<>:mydate';
Q.ParamByName('mydate').AsDate := now;
Writeln(sLineBreak, '1: ', Q.Fields.AsString, sLineBreak);
Q.ParamByName('mydate').AsDate := StrToDate('17.10.2017');
Writeln('2: ', Q.Fields.AsString);
3.) Check the output - you'll see that the SQL is parsed several times and the statement is prepared all over again.
4.) Uncomment the "FPrepared := True;" line in the patch.
5.) See again, the needed prepare/format calls have reduced.
|Additional Information||The patch includes Writeln debug lines. Please delete them before applying to FPC.|
|Tags||No tags attached.|
|Fixed in Revision||37951|
mssql-prepare-01.patch (1,178 bytes)
Index: packages/fcl-db/src/base/dsparams.inc =================================================================== --- packages/fcl-db/src/base/dsparams.inc (revision 37897) +++ packages/fcl-db/src/base/dsparams.inc (working copy) @@ -303,6 +303,7 @@ tmpParam:TParam; begin + Writeln('TParams.ParseSQL: ', Copy(SQL, 1, 20)); // DEBUG if DoCreate then Clear; // Parse the SQL and build ParamBinding ParamCount:=0; Index: packages/fcl-db/src/sqldb/mssql/mssqlconn.pp =================================================================== --- packages/fcl-db/src/sqldb/mssql/mssqlconn.pp (revision 37897) +++ packages/fcl-db/src/sqldb/mssql/mssqlconn.pp (working copy) @@ -225,10 +225,12 @@ var ParamBinding : TParamBinding; begin + Writeln('TDBLibCursor.Prepare - ParseSQL: ', Copy(Buf, 1, 20)); // DEBUG if assigned(AParams) and (AParams.Count > 0) then FQuery:=AParams.ParseSQL(Buf, false, sqEscapeSlash in FConnection.ConnOptions, sqEscapeRepeat in FConnection.ConnOptions, psSimulated, ParamBinding, FParamReplaceString) else FQuery:=Buf; + FPrepared := True; end; function TDBLibCursor.ReplaceParams(AParams: TParams): string;
mssql-prepare-01.patch (1,178 bytes)
mssqlconn uses dblib to talko to sql server. dblib does not provide server-side statement preparation. So PrepareStatement only localy parses SQL and substitute parameters with parameters placeholders.
There is also "hack" in sqldb, which calls UnprepareStatement when dataset is closed and statement is NOT prepared. This allows do cleaning of "in process resultset". There is "feature" in MSSQL, that when not all rows of pending resultset are consumed then next attempt to send another sql command ends with error 20019 : Attempt to initiate a new Adaptive Server operation with results pending
So I am not sure if this patch cannot cause compatibility issues.
||@Laco, can you explain where this 'Hack' is ? In TSQLQuery ?|
> There is also "hack" in sqldb, which calls UnprepareStatement when dataset is closed and statement is NOT prepared.
@Laco: yes and no. Actually it's UnPrepared anyway
1.) procedure TCustomSQLQuery.InternalClose;
-->> if not Prepared then FStatement.DoUnprepare;
2.) procedure TCustomSQLQuery.SetActive(Value: Boolean);
-->> if not Value and IsPrepared then UnPrepare;
You see once there is a check for IsPrepared and once there is a check for not IsPrepared.
I assume such a problem will be registered quickly and then you can revert the patch.
If so, this is wrong.
The TSQLQuery should prepare if needed, but should only unprepare if it prepared itself.
The idea is that
Q.SQL.Text:='Select Blah from MyTable where Id=:ID';
For something in myloop do
// Aloha data
is prepared only once, and then executed repeatedly.
||@Michael: exactly, this is what I wanted to look into after this patch is applied.|
As Ondrej pointed out. There seems not to be a problem adding FPrepared:=True; only in TMSSQLConnection.
But going deeper into sqldb may lead to problems. There is for example from begining comment in:
// This is only necessary because TIBConnection can not re-open a
// prepared cursor. In fact this is wrong, but has never led to
// problems because in SetActive(false) queries are always
// unprepared. (which is also wrong, but has to be fixed later)
if IsPrepared then with SQLConnection do
Probably if fixed sqldb, then it may lead to need update applications and use:
SQLQuery1.Unprepare; // cancel pending rows if there are any
On the other hand, probably what is now in TMSSQLConnection handled in UnPrepareStatement should be moved to FreeFldBuffers (as far as FreeFldBuffers is called by InternalClose and here we can cancel pending resultset if client DataSet is closed)
IMO any change in sqldb should be checked at least using dbtestframework against all supported databases.
(I can fix MSSQLConnection and run test suite to do quick check)
We definitely need to dig deeper in SQLDb
1. As said, TSQLQuery may only unprepare if it has prepared by itself.
Otherwise the whole idea of
"prepare once, execute many times" is simply thrown away.
It is the whole point of having prepare, after all.
2. The BeforeRefreshOpenCursor is plain wrong.
If Firebird cannot handle this situation,
it needs to close and re-execute the query.
3. The MSSQL thing is separate.
It needs to be fixed first, and then the deeper sqlDB issue must be checked.
I have applied the patch, but removed the debug statements.
For the issue if prepare/unprepare by TSQLQuery, I think it's best to create a separate bugreport, it is a separate issue after all.
|2018-01-12 11:53||Ondrej Pokorny||New Issue|
|2018-01-12 11:53||Ondrej Pokorny||File Added: mssql-prepare-01.patch|
|2018-01-12 12:31||Michael Van Canneyt||Assigned To||=> Michael Van Canneyt|
|2018-01-12 12:31||Michael Van Canneyt||Status||new => assigned|
|2018-01-12 15:49||LacaK||Note Added: 0105698|
|2018-01-12 15:49||LacaK||Status||assigned => feedback|
|2018-01-12 15:57||Michael Van Canneyt||Note Added: 0105699|
|2018-01-12 16:25||Ondrej Pokorny||Note Added: 0105702|
|2018-01-12 16:25||Ondrej Pokorny||Status||feedback => assigned|
|2018-01-12 16:40||Michael Van Canneyt||Note Added: 0105704|
|2018-01-12 16:50||Ondrej Pokorny||Note Added: 0105706|
|2018-01-12 18:47||LacaK||Note Added: 0105713|
|2018-01-12 18:57||Michael Van Canneyt||Note Added: 0105716|
|2018-01-13 10:58||Michael Van Canneyt||Fixed in Revision||=> 37951|
|2018-01-13 10:58||Michael Van Canneyt||Note Added: 0105730|
|2018-01-13 10:58||Michael Van Canneyt||Status||assigned => resolved|
|2018-01-13 10:58||Michael Van Canneyt||Fixed in Version||=> 3.1.1|
|2018-01-13 10:58||Michael Van Canneyt||Resolution||open => fixed|
|2018-01-13 10:58||Michael Van Canneyt||Target Version||=> 3.2.0|
|2018-01-14 10:29||Ondrej Pokorny||Note Added: 0105781|
|2018-01-14 10:29||Ondrej Pokorny||Status||resolved => closed|