0019343: UnPrepare should be called when Prepared in procedure TCustomSQLQuery.InternalClose - Free Pascal/Lazarus Bug Tracker

View Issue Details [ Jump to Notes ]

[ Issue History ] [ Print ]

ID

Project

Category

View Status

Date Submitted

Last Update

0019343

FPC

Database Components

public

2011-05-12 09:28

2012-05-14 07:27

Reporter

LacaK2

Assigned To

Joost van der Sluis

Priority

normal

Severity

trivial

Reproducibility

have not tried

Status

feedback

Resolution

open

Platform

OS

OS Version

Product Version

2.4.3

Product Build

Target Version

Fixed in Version

Summary

0019343: UnPrepare should be called when Prepared in procedure TCustomSQLQuery.InternalClose

Description

There is IMHO reversed logic on lines:

1151 if (not IsPrepared) and (assigned(database)) and (assigned(FCursor)) then TSQLConnection(database).UnPrepareStatement(FCursor);

1411 if (not IsPrepared) and (assigned(database)) and (assigned(FCursor)) then TSQLConnection(database).UnPrepareStatement(Fcursor);

UnPrepare should be called when statement *IsPrepared* not when *not IsPrepared*

Tags

No tags attached.

FPCOldBugId

Fixed in Revision

Relationships

Notes
(0048230) Joost van der Sluis (developer) 2011-05-12 10:43 edited on: 2011-05-12 10:46	Try it and run the full testsuite. Getting this right was quite a puzzle. You can also use 'svn blame' and read the commit-messages. Maybe they give some more information. And, if this is a bug, you can provide an example which shows that something goes wrong. Looking at code only, spotting a bug without being able to show the problem, doesn't make it a bug. (IsPrepared does not what you think it does) So, please show me what the actual problem is. I don't see any right now.

(0048414) LacaK2 (reporter) 2011-05-19 15:05 edited on: 2011-05-20 08:24	Please look at this revision (there was introduced (not IsPrepared)): http://svn.freepascal.org/cgi-bin/viewvc.cgi/trunk/packages/fcl-db/src/sqldb/sqldb.pp?r1=12284&r2=12427 [^] I can show problem on ExecSQL method, where was used ExecDidPrepare variable, which saved state of cursor BEFORE Prepare was called. (It is clear, that state changes after call to Prepare to IsPrepared) And this variable was used after call to Execute ... if cursor was prepared before entering ExecSQL method then leave it prepared else unprepare. In current implementation unprepare is never called because cursor is always prepared (thanks to unconditional call to Prepare). So now when leaving ExecSQL, cursor always remains prepared, regardless of whether prior was Prepared or not. So we must say what is purpose of this command?: if (not IsPrepared) and (assigned(database)) and (assigned(FCursor)) then TSQLConnection(database).UnPrepareStatement(Fcursor); 1. original purpose was unprepare after execute if was not prepared before ... if we want this then we must change code 2. or another meaning can be unprepare if some exceptions occurs during prepare call (but what unprepare if prepare does not completed) ... if we wan this then we must change at least comment lines to reflect this

(0051409) Marco van de Voort (manager) 2011-09-02 23:27	bump

(0059250) Marco van de Voort (manager) 2012-05-05 11:47	Bump with feedback, year old again.

(0059482) LacaK2 (reporter) 2012-05-11 14:49 edited on: 2012-05-11 15:03	I ran test suite with "not IsPrepared" changed to "IsPrepared" for SQLite, Firebird and MSSQL. For Firebird there appears failure: <test name="TTestFieldTypes.TestRowsAffected"> <failure ExceptionClassName="EAssertionFailedError"> <message> expected: <1> but was: <-1></message> </failure> </test> It is because after ExecSQL, statement is unprepared and when it is once unprepared we can not get "records affected" of unprepared statement. So we can not change it as easy as I wrote. But question remains: what is purpose of this command: if (not IsPrepared) and (assigned(database)) and (assigned(FCursor)) then TSQLConnection(database).UnPrepareStatement(Fcursor); I read it as "unprepare" when is "unprepared". I do not see logic in it ;-) When I comment it in procedure TCustomSQLQuery.ExecSQL and change "not IsPrepared" to "IsPrepared" in procedure TCustomSQLQuery.InternalClose then tests pass ok and also it seems to me as logical solution ;-)

(0059574) LacaK2 (reporter) 2012-05-14 07:27	Thinking more about subject, there can be also other interpretation. When error occurs during preparation stage, then IsPrepared is false. So testing if (not Prepared) can be read as "if something went wrong during preparation" then do "clean up" by calling UnPrepare. If this is intention, then adding comment will be sufficient and we can close this bug report.

Issue History
Date Modified	Username	Field	Change
2011-05-12 09:28	LacaK2	New Issue
2011-05-12 09:28	LacaK2	Status	new => assigned
2011-05-12 09:28	LacaK2	Assigned To	=> Joost van der Sluis
2011-05-12 10:43	Joost van der Sluis	Note Added: 0048230
2011-05-12 10:46	Joost van der Sluis	Note Edited: 0048230
2011-05-19 15:05	LacaK2	Note Added: 0048414
2011-05-20 08:24	LacaK2	Note Edited: 0048414
2011-09-02 23:27	Marco van de Voort	Note Added: 0051409
2012-05-05 11:47	Marco van de Voort	Note Added: 0059250
2012-05-05 11:47	Marco van de Voort	Status	assigned => feedback
2012-05-11 14:49	LacaK2	Note Added: 0059482
2012-05-11 15:03	LacaK2	Note Edited: 0059482
2012-05-14 07:27	LacaK2	Note Added: 0059574

MantisBT 1.2.12[^]

Copyright © 2000 - 2012 MantisBT Group