Suggestion to reduce errors caused by reusing IBM_DBStatement variables

[imavo]

Is your feature request related to a problem? Please describe.

See the sample code-fragment below that demonstrates the problem (without the workaround). However, any programmer that calls any ibm_db API that takes an input argument of an IBM_DBStatement object can get a variety of errors (depending on which ibm_db API gets invoked, as long as it needs an input IBM_DBStatement object) if the same variable gets re-used for the IBM_DBStatement object.

Describe the solution you'd like
To Reduce the chances of errors resulting from re-using variables for IBM_DBStatement objects ibm_db.free_result() shhould conditionally free the cli-statement-handle and after freeing that cli-statement-handle should remove its identity from the IBM_DBStatement object (for example by setting the hstmt to (SQLHSTMT) -1 ). In other words, instead of simply closing the cursor ,it frees the cli-statement-handle also.

I note that the destructor function already conditionally does the SQLFreeHandle() when the hstmt is not -1.

This suggestion will benefit those you use ibm_db.free_result(), but will not help those who fail to use it when it needs to be used and who also do not use the workaround.

The details of the suggestion is to change the code for ibm_db_free_result function to read as follows:

static PyObject *ibm_db_free_result(PyObject *self, PyObject *args)
{
    PyObject *py_stmt_res = NULL; 
    stmt_handle *stmt_res;
    int rc = 0;

    if (!PyArg_ParseTuple(args, "O", &py_stmt_res))
        return NULL; 

    if (!NIL_P(py_stmt_res)) {
        if (!PyObject_TypeCheck(py_stmt_res, &stmt_handleType)) {
            PyErr_SetString( PyExc_Exception, "Supplied statement object parameter is invalid" );
            return NULL; 
        } else {
            stmt_res = (stmt_handle *)py_stmt_res;
        }
        if ( stmt_res->hstmt != (SQLHSTMT)-1 ) {
            /* Free any cursors that might have been allocated in a previous call
            * to SQLExecute, and drop the cli-statement handle.
            */
            Py_BEGIN_ALLOW_THREADS;
            rc = SQLFreeHandle( SQL_HANDLE_STMT, (SQLHSTMT)stmt_res->hstmt);
            Py_END_ALLOW_THREADS;
            if ( rc == SQL_ERROR ||  rc == SQL_SUCCESS_WITH_INFO )
            {
               _python_ibm_db_check_sql_errors( stmt_res->hstmt, SQL_HANDLE_STMT,
                                                rc, 1, NULL, -1, 1);
            }
            stmt_res->hstmt = (SQLHSTMT)-1;
            if ( rc == SQL_ERROR ) {
                PyErr_Clear( );
                Py_RETURN_FALSE;
            }
        }
        _python_ibm_db_free_result_struct(stmt_res);
    } else {
        PyErr_SetString(PyExc_Exception, "Supplied parameter is invalid");
        return NULL; 
    }
    Py_INCREF(Py_True);
    return Py_True;
}

Describe alternatives you've considered
It is possible to avoid errors by ensuring that the programmer always remembers to assign "None" to any re-used variable that will store an IBM_DBStatement object before reassigning such a variable. This is considered non-pythonic and it is not mentioned in the wiki to be a requirement, and is easily avoided if the ibm_db code was altered as suggested above.

Additional context
This problem was mentioned in a different guise in #830

Sample python script demonstrating the problem (may others are possible).

import ibm_db
import sys 


schema="..."
tablename=None
authid=schema
token="..."
databases_list=["..."]

for i in range(2):
    for db in databases_list:
        print(f"Database {db} Iteration {i} is starting...\n")
        conn = ibm_db.connect( db, authid, token )
        if not conn:
            print(f"Error failed to connect to database {db} :{ibm_db.conn_errormsg()}\n")
            sys.exit(1)
    
        try:
            stObj1=ibm_db.columns(conn, None, None, '', None)
            if stObj1 is False:
                print(f"Unexpected failure of ibm_db.columns\n{ibm_db.stmt_errormsg()}\n")
                sys.exit(1)
            row=ibm_db.fetch_both(stObj1)
            # only print the first row in result-set 
            if (row):
                print(f"{row['TABLE_SCHEM']}.{row['TABLE_NAME']}.{row['COLUMN_NAME']} {row['TYPE_NAME']}\n")
            else:
                print(f"No matching rows\n")

    
        except Exception as e:
            print(f"Database {db} Iteration {i}: \n{ibm_db.stmt_errormsg()}")
            raise e

        ibm_db.rollback(conn)
        ibm_db.free_result(stObj1)
        ibm_db.close(conn)
        conn=None


sys.exit(0)

[Earammak]

Hello @imavo,
The changes you are suggested in ibm_db_free result is already handled from function ibm_db_free_stmt.
And for your sample program if you use ibm_db.free_stmt API https://github.com/ibmdb/python-ibmdb/wiki/APIs#ibm_dbfree_stmt instead of ibm_db.free_result, it will work without any problem.

Thanks

[imavo]

The advantage of a statement-object is that it allows re-use (for example , in a loop). Are you suggesting that each loop iteration should use ibm_db.free_stmt() ? Why should'nt the ibm_db.free_result() (at the end of processing a result-set) allow the programmer to work in a more pythonic manner by re-using statement-variables, which would be possible with my suggestion. I cannot see a logical reason why ibm_db.free_result() does not already do what I suggest above. Can you give a logical reason for not closing the CLI-handle as suggested?

[Earammak]

Whatever you suggested to change the function, already exists in ibm_db_free_stmt function.
The purpose of ibm_db_free_result is to close the existing cursor and can use the same statement handle to execute other queries. when statement handle is dropped in the ibm_db_free_result, will not be able to execute queries in the same statement handle.
Function ibm_db_free_result use "SQLFreeStmt((SQLHSTMT)stmt_res->hstmt, SQL_CLOSE)"which means that the cursor will be closed. And ibm_db_free_stmt will use "SQLFreeHandle( SQL_HANDLE_STMT, handle->hstmt)", which means that it frees the statement handle.