New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EXECUTE BLOCK statement and ISC_DSQL_EXECUTE2() problem. [CORE3680] #4030
Comments
Commented by: @hvlad What happens if you |
Commented by: @jasonwharton Vlad, a) It works just great! No cursor seems to be left open that causes a problem. My conclusion is the Firebird engine is opening a cursor by mistake. |
Commented by: @hvlad For (a) - are you sure you don't call isc_dsql_free_stateement() for SELECT statement ? |
Commented by: @jasonwharton Yes, I'm positive because I had to take code from the TIB_Dataset class level and bring it to the TIB_Statement level so that I could implement this workaround to explicitly close the cursor after executing the statement that I'll show you here: procedure TIB_Statement.API_Execute2; I never needed to do this before. |
Commented by: @hvlad Well, seems i found where SELECT is different from selectable EXECUTE BLOCK. SELECT statement is considered as singleton if it is executed using isc_dsql_execute2() with not NULL output parameters. The question is : should we interpret EXECUTE BLOCK as singleton in the same conditions... |
Commented by: @jasonwharton The issue here seems to be a question of what behavior should the isc_dsql_execute2() call have. I don't think it is appropriate under any circumstances to automatically create a cursor. The purpose of a statement such as this is to efficiently execute over and over again without the necessity for chatter on the wire to close cursor, open cursor, fetch record, etc. Hopefully this can be improved such that the EXECUTE BLOCK will have the same behavior as a regular SELECT statement. Also, I'm curious about the NOT NULL or NULL columns having an influence on something being handled as a singleton or not. This seems like something that should not have an influence. Perhaps you can explain that a little more? Thanks! |
Commented by: @hvlad > Also, I'm curious about the NOT NULL or NULL columns having an influence on something being handled as a singleton or not. This seems like something that should not have an influence. Perhaps you can explain that a little more? Thanks! It was not about SQL NULL's :) It was about value of last parameter of isc_dsql_execute2() call (namely out_sqlda). Inside the engine we have no separate isc_dsql_execute() and isc_dsql_execute2() calls. There is one common routine with signature of isc_dsql_execute2(). When last parameter (out_sqlda) is not set by user application (i.e. NULL\nil was passed into isc_dsql_execute2() or isc_dsql_execute() was used) then call When out_sqlda is non-empty, call is considered as singleton for SELECT statement, but (wrongly?) not for EXECUTE BLOCK statement. Hope it is clear now. |
Modified by: @hvladassignee: Vlad Khorsun [ hvlad ] |
Commented by: @jasonwharton Ok, your explanation makes sense. This is how it tells the difference between isc_dsql_execute() and isc_dsql_execute2(). If there is an Out_DA parameter then it takes it as a singleton. Do you agree in this case the EXECUTE BLOCK statement should also be treated as a singleton? Will this be difficult to fix? |
Commented by: Sean Leyne (seanleyne) I do not believe that EXECUTE BLOCK should be treated as a singleton. I have created EXECUTE BLOCK which has returned a set of rows (a simple one time report based on complex logic). |
Commented by: @asfernandes While on that, I have some memories that in old days, execute block with output parameters and without suspend (as below) was considered as singleton and returned one row in isql. But it don't now. Was this changed or am I wrong? execute block returns (n integer) |
Commented by: @jasonwharton Sean, EXECUTE BLOCK wouldn't be treated as a singleton in all cases. This is only in the case where it is executed by the isc_dsql_execute2() API call. The point here is to get consistent behavior for all calls to isc_dsql_execute2() such that it will deliver a single row of output only or return an exception and also to not have any cursor created on the server. This would cause it to behave just like a regular SELECT statement when using this API call. The change I am requesting would not prevent you from opening a cursor and using it exactly as you have always used it in the past. Does this help clarify what I am asking or are you still not clear? |
Commented by: @jasonwharton Adriano, I can't answer your question directly because I don't know how all of the internals work. This is an area of working with Firebird that I would like clarified. If SUSPEND is not called and a cursor has been opened up for it then it should be treated as an empty result set. This is how I request this be handled so that there is versatility in dealing with singleton's within the existing architecture of Firebird. |
Commented by: @hvlad Jason, > Do you agree in this case the EXECUTE BLOCK statement should also be treated as a singleton? > Will this be difficult to fix? |
Commented by: @hvlad Adriano, IIRC, initial implementation was changed at v2.5 to be more consistent with more strict rules of usage of SUSPEND in PSQL |
Commented by: @hvlad The fix is committed into v2.5.2 |
Commented by: @jasonwharton Ok, I got the latest snapshot and had a look at things. This statement works perfectly. EXECUTE BLOCK However, this statement returns the error: EXECUTE BLOCK When you are using a statement in the isc_dsql_execute2() mode, should it require a suspend whenever data is returned? In the past the procedure was treated as a singleton such that the use of SUSPEND was not required. In my opinion, even if Firebird departs a bit from the standards, I think there is value in maintaining legacy compatibility. I suggest that if a SUSPEND is not in the body of the procedure block that its output results be returned as a singleton record automatically. Kind of like an implicit SUSPEND if the programmer neglects putting it in. Also, this should be sensitive to the API call being made. It should only apply the implicit SUSPEND when the execute2() singleton context exists. |
Commented by: @hvlad Create procedure CREATE PROCEDURE P1 and try to excecute SELECT * FROM P1 using isc_dsql_execute2(). The rule about EXECUTE BLOCK is : The principal difference between selectable and non-selectable procedures (and blocks) is that selectable ones returns records when SUSPEND is encountred *and* one more record on exit. The singleton check, performed inside the engine, have the following logic: It is not possible to change generated code for EXECUTE BLOCK when execution is started and engine know about presence of singleton check. And, yes, selectable EXECUTE BLOCK *must* have SUSPEND inside. |
Commented by: @jasonwharton Vlad, Thank for your giving the detailed explanation of the server internals. If I understand you correctly, there isn't a time when an EXECUTE BLOCK with output parameters will work properly if there isn't a SUSPEND in the body of it. Wouldn't it be possible to detect that there wasn't a SUSPEND token in the body of an EXECUTE BLOCK (with output parameters) and therefore to automatically append an implicit SUSPEND at the end of it? This way the values of the records for output will be appropriately passed on as a singleton select. (I realize by now this is merely a request for enhancement and that you have addressed the issue of this entry 100% with what you have already done.) Note: I'll go ahead and make an enhancement request so that this ticket can be closed out as resolved. Big THANK YOU for your efforts here! |
Submitted by: @jasonwharton
It seems that Firebird 2.5 has an issue where repeated executions of an
EXECUTE BLOCK statement are not possible via ISC_DSQL_EXECUTE2() due to the
error:
"Attempt to reopen an open cursor"
Here is a trace in IB Objects (using a TIB_DSQL component) of what happened:
/*---
ALLOCATE STATEMENT
DB_HANDLE = 1
STMT_HANDLE = 188
----*/
/*---
PREPARE STATEMENT
TR_HANDLE = 186
STMT_HANDLE = 188
EXECUTE BLOCK
RETURNS (I INTEGER)
AS
BEGIN
SELECT FIRST 1 MON$ATTACHMENT_ID FROM MON$ATTACHMENTS
INTO :I;
EXIT;
END
PLAN (MON$ATTACHMENTS NATURAL)
FIELDS = [ Version 1 SQLd 1 SQLn 30
I = <NIL> ]
SECONDS = 0.016
----*/
/*---
STATEMENT INFO
STMT_HANDLE = 188
----*/
/*---
STATEMENT INFO
STMT_HANDLE = 188
----*/
/*---
EXECUTE2 ROW SINGLETON
TR_HANDLE = 186
STMT_HANDLE = 188
PARAMS = [ ]
FIELDS = [ Version 1 SQLd 1 SQLn 1
I = 53765 ]
----*/
/*---
EXECUTE2 ROW SINGLETON
TR_HANDLE = 186
STMT_HANDLE = 188
PARAMS = [ ]
FIELDS = [ Version 1 SQLd 1 SQLn 1
I = 53765 ]
ERRCODE = 335544569
----*/
/*---
INTERPRET BUFFER =
ERRCODE = 17
----*/
/*---
INTERPRET BUFFER = Dynamic SQL Error
ERRCODE = 21
----*/
/*---
INTERPRET BUFFER = SQL error code = -502
ERRCODE = 32
----*/
/*---
INTERPRET BUFFER = Attempt to reopen an open cursor
ERRCODE = -1
----*/
/*---
COMMIT
TR_HANDLE = 186
----*/
The call shown as EXECUTE2 ROW SINGLETON is the isc_dsql_execute2() API call.
Thanks to anyone who can do a low-level verification of this. It seems to me there shouldn't be an error here.
Thanks,
Jason Wharton
Commits: 58ec672
The text was updated successfully, but these errors were encountered: