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


##########
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:
   Quoting sqlalchemy:
   
   - about `CursorResult.return_rows` "Overall, the value of 
[CursorResult.returns_rows](https://docs.sqlalchemy.org/en/14/core/connections.html?highlight=returns_#sqlalchemy.engine.CursorResult.returns_rows)
 should always be synonymous with whether or not the DBAPI cursor had a 
.description attribute, indicating the presence of result columns, noting that 
a cursor that returns zero rows still has a .description if a row-returning 
statement was emitted."
   - about `row_count` (aforementioned link) "Statements that use RETURNING may 
not return a correct rowcount."
   - about `row_count` (aforementioned link) "Contrary to what the Python DBAPI 
says, it does not return the number of rows available from the results of a 
SELECT statement as DBAPIs cannot support this functionality when rows are 
unbuffered."
   
   Quoting PEP-249:
   
   - about `cursor.rowcount` https://peps.python.org/pep-0249/#id48 "The term 
number of affected rows generally refers to the number of rows deleted, updated 
or inserted by the last statement run on the database cursor."
   - about `cursor.description` https://peps.python.org/pep-0249/#description 
"This attribute will be None for operations that do not return rows or if the 
cursor has not had an operation invoked via the 
[.execute*()](https://peps.python.org/pep-0249/#id14) method yet."



-- 
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