nim-lang / db_connector

Unified db connector in Nim
MIT License
20 stars 6 forks source link

db_odbc - invalid cursor state error when calling stored procedures #4

Open alistairkeys opened 2 years ago

alistairkeys commented 2 years ago

db_odbc.exec (and probably the other variations of exec) raises an 'Invalid Cursor State' error when attempting to call a stored procedure as it tries to fetch results as the last step and stored procedures won't have data to fetch (a 'stored procedure' returns nothing while a 'stored function' returns something).

I couldn't see anything related to stored procedures (or stored procedures within packages in my case as I'm mucking around in Oracle) within db_odbc so I tried the exec function and it didn't work. Apologies if I've just missed a relevant API.

I think this is a generic ODBC consideration rather than being vendor-specific.

Example

-- stored proc for Oracle
create or replace procedure boom as
begin
  null; -- PL/SQL's equivalent of Nim's discard statement
end boom;
/
import std/db_odbc
var dbConn = open("foo", "bar", "baz", "qux")
try:
  dbConn.exec sql"{ call boom() }"
finally:
  dbConn.close

Current Output

Error: 24000 [Oracle][ODBC]Invalid cursor state.
db_odbc.nim(168)         dbError
Error: unhandled exception: ODBC Error [DbError]

Expected Output

No output (but no error)

Possible Solution

I found documentation discussing the 'Invalid Cursor State' error:

A cursor was positioned on the StatementHandle by SQLFetch or SQLFetchScroll. This error is returned by the Driver Manager if SQLFetch or SQLFetchScroll has not returned SQL_NO_DATA, and is returned by the driver if SQLFetch or SQLFetchScroll has returned SQL_NO_DATA.

The same sort of comment exists in the SQLExecDirect docs too.

I can see logic like the following in db_odbc:

db.sqlCheck(SQLExecDirect(db.stmt, q.PSQLCHAR, q.len.TSqlSmallInt))
db.sqlCheck(SQLFetch(db.stmt))

# ... and 

db.sqlCheck(SQLExecute(db.stmt))
result = SQLFetch(db.stmt)
db.sqlCheck(result)

It seems the API doesn't have a way to say "don't do a fetch at the end". I imagine the solution would be some combination of:

  1. Add a 'returns stuff' bool in relevant calls, defaults to true
  2. Add a new 'execProc'/'execWithoutResults'/'execBikeshed' procedure that does the same thing as the existing logic but doesn't try to fetch anything
  3. Come up with some abstraction for stored procedures (this may exist but I didn't spot anything in db_odbc)

For what it's worth, I tried a copy-paste-tweak version of exec with the SQLFetch commented out and it appeared to work when calling stored procedures. (The SQLExecute call returned 0 (SQL_SUCCESS).)

Additional Information

Nim Compiler Version 1.6.2 [Windows: amd64]
Compiled at 2021-12-17
Copyright (c) 2006-2021 by Andreas Rumpf

active boot switches: -d:release

(but this has been an issue for ages.)

Windows 10 / Oracle instant client 19.

whee commented 2 years ago

The SQLExecute call returned 0 (SQL_SUCCESS)

This is the part that confuses me a bit, although I'm new to ODBC. If I SQLDirectExec a statement such as SET NOCOUNT ON (just an example, can be anything with no results), I get SQL_SUCCESS. I am expecting a SQL_NO_DATA out of this; otherwise, how is the ODBC wrapper supposed to know if SQLFetch is necessary?

I've worked around this locally with a "fetch" parameter that decides whether or not to fetch, but it would be better to have prepareFetchDirect know if a fetch is necessary. I'm just not sure how to get the API to tell me this.

Another thing I have tried, and which seems to work, is this:

proc prepareFetchDirect(db: var DbConn, query: SqlQuery,
                args: varargs[string, `$`]) {.
                tags: [ReadDbEffect, WriteDbEffect], raises: [DbError].} =
  # Prepare a statement, execute it and fetch the data to the driver
  # ready for retrieval of the data
  # Used internally by iterators and retrieval procs
  # requires calling
  #      properFreeResult(SQL_HANDLE_STMT, db.stmt)
  # when finished
  db.sqlCheck(SQLAllocHandle(SQL_HANDLE_STMT, db.hDb, db.stmt))
  var
    q = dbFormat(query, args)
  db.sqlCheck(SQLExecDirect(db.stmt, q.PSQLCHAR, q.len.TSqlSmallInt))

  var
    c: TSqlSmallInt = 0

  db.sqlCheck(SQLNumResultCols(db.stmt, c))
  if c > 0:
    db.sqlCheck(SQLFetch(db.stmt))

Basically, check if there are results with SQLNumResultCols before deciding whether or not to fetch.

sdmcallister commented 1 year ago

Adding an example from mssql that might be related. In the example below, the data is inserted into the table so the error occurs afterwards. getAllRows isn't executed.

import std/db_odbc
var db = open("mssql1", "", "", "")
echo "opened connection!"

db.exec(sql"INSERT INTO [localdw].[dbo].[myints](myints) VALUES (42)")

echo getAllRows(db,sql"SELECT * FROM myints")

db.close()
echo "connection closed."

Error output:


Warning: 01000 [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Changed database context to 'localdw'.
opened connection!

Error: 24000 [Microsoft][ODBC Driver 17 for SQL Server]Invalid cursor state
C:\nim_projects\timer\testmssql.nim(4) testmssql
C:\nim-1.6.12\lib\impure\db_odbc.nim(261) exec
C:\nim-1.6.12\lib\impure\db_odbc.nim(237) prepareFetchDirect
C:\nim-1.6.12\lib\impure\db_odbc.nim(172) sqlCheck
C:\nim-1.6.12\lib\impure\db_odbc.nim(168) dbError
Error: unhandled exception: ODBC Error [DbError]
Error: execution of an external program failed: 'C:\nimcache\testmssql_d\testmssql_6013DC0591D9424B9B314E815008335FB14A37B1.exe '