mkleehammer / pyodbc

Python ODBC bridge
https://github.com/mkleehammer/pyodbc/wiki
MIT No Attribution
2.92k stars 561 forks source link

Do we need to close cursor in this case? #1366

Closed nishantvarma closed 3 months ago

nishantvarma commented 3 months ago

I have a code that uses cursor directly like this (slightly modified version of Products.SQLAlchemyDA.query):

def execute(session, query):
    connection = session.connection().connection
    cursor = connection.cursor()
    cursor.execute(query)
    description = cursor.description
    if description is not None:
        rows = cursor.fetchall()
    return rows

result = execute(session, "select 1")

I get "Connection is busy" errors when there is heavy load in the server. However, I don't see a possibility of this as:

1) I use single session per thread. 2) I always consume the results. 3) Cursor gets closed after it goes out of scope due to GC?

Is it better to use the closing context-manager in this case? There are some edge cases where fetchall might not get called (code not shown, presume that is not causing this).

def execute(session, query):
    connection = session.connection().connection
    with closing(connection.cursor()) as cursor:
        cursor.execute(query)
        description = cursor.description
        if description is not None:
            rows = cursor.fetchall()
    return rows

result = execute(session, "select 1")

Kindly let me know your thoughts.

v-chojas commented 3 months ago

Are you also making one connection to the database per thread?

gordthompson commented 3 months ago

Just to be clear: session is a SQLAlchemy Session object, correct?

nishantvarma commented 3 months ago

Thanks for checking, @v-chojas and @gordthompson. I forgot to add a small detail (and that is related to what you are asking). Usually, the code flow is like this:

def api():
    result = session.query(Table) # orm
    result = session.execute(query) # sql
    result = execute(session, query) # legacy
    ...

where session is SQLAlchemy's scoped session, to execute queries. I think it uses the same connection because it uses the session.connection() pool under the hood. This has been my observation so far. Are there any problems mixing like this? (PS: I have been recommended to use cursor for complex queries.)

v-chojas commented 3 months ago

I don't know what driver or DB you're using, but some don't support having more than one simultaneous active resultset on the same connection.

nishantvarma commented 3 months ago

I use ODBC 17. Is there is a possibility of multiple active resultsets in this case? I thought these queries are sequential. Enabling MARS is the last choice; would prefer to prevent it if possible. Should I use the closing context manager to be on the safe side?

v-chojas commented 3 months ago

You mention multiple threads and "heavy load".

nishantvarma commented 3 months ago

Yeah, but I am wondering if it is possible for the same connection to use multiple cursors in this case because: 1) scoped session returns the same connection always; 2) sequentially executing queries from the same connection can't overlap.

I.e., this will never happen:

conn = session.connection().connection
result = conn.cursor().execute("SELECT 1")
conn.cursor().execute("SELECT 1")

result will always be consumed after execution.

I am already aware of exceptions causing a miss to cursor.fetchall; but this one seems different.

v-chojas commented 3 months ago

I recommend you think more about what it means to have multiple threads and why it only happens under "heavy load".

nishantvarma commented 3 months ago

I will read more on that. Appreciate your help!