FanatoniQ commented on code in PR #25430:
URL: https://github.com/apache/airflow/pull/25430#discussion_r934213608


##########
airflow/providers/common/sql/hooks/sql.py:
##########
@@ -30,13 +30,10 @@
 from airflow.utils.module_loading import import_string
 from airflow.version import version
 
-if TYPE_CHECKING:
-    from sqlalchemy.engine import CursorResult
 
-
-def fetch_all_handler(cursor: 'CursorResult') -> Optional[List[Tuple]]:
+def fetch_all_handler(cursor) -> Optional[List[Tuple]]:
     """Handler for DbApiHook.run() to return results"""
-    if cursor.returns_rows:
+    if cursor.description is not None:
         return cursor.fetchall()

Review Comment:
   @uranusjr  This doesn't look true to me, I am using the following as 
reference:
   
   - https://peps.python.org/pep-0249/#description
   - 
https://docs.sqlalchemy.org/en/14/core/connections.html?highlight=returns_#sqlalchemy.engine.CursorResult.returns_rows
   
   Also:
   
   ```python
   >>> import pymssql
   >>> c = pymssql.connect(host, login, password)
   >>> cur = c.cursor()
   >>> cur.execute("SELECT SUSER_SNAME();")
   >>> cur.rowcount
   -1
   >>> cur.description
   (('', 1, None, None, None, None, None),)
   >>> cur.execute("PRINT('1');")
   >>> cur.rowcount
   -1
   >>> cur.description
   ```
   
   Edit: I have the same behaviour with `sqlite3` and `jaydebeapi`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to