kazanzhy commented on code in PR #23971:
URL: https://github.com/apache/airflow/pull/23971#discussion_r922847638


##########
airflow/providers/common/sql/hooks/sql.py:
##########
@@ -232,22 +270,19 @@ def run(self, sql, autocommit=False, parameters=None, 
handler=None):
                 results = []
                 for sql_statement in sql:
                     self._run_command(cur, sql_statement, parameters)
+
                     if handler is not None:
                         result = handler(cur)
                         results.append(result)
 
-            # If autocommit was set to False for db that supports autocommit,
-            # or if db does not supports autocommit, we do a manual commit.
+            # If autocommit was set to False or db does not support 
autocommit, we do a manual commit.
             if not self.get_autocommit(conn):
                 conn.commit()
 
         if handler is None:
             return None
-
-        if scalar:
-            return results[0]
-
-        return results
+        else:

Review Comment:
   I think in the very first iteration it was `return results[-1]` which good 
for a single statement and list of them.
   In one of our previous PRs, we got the idea to return all statement's 
results.
   https://github.com/apache/airflow/issues/23623#issuecomment-1122758517
   In this case, we will have the next returning type 
`Optional[List[HandlerResult]]`. 
   Before that, it was `Optional[HandlerResult]`. We could add parameter 
`return_last=True` and then we will have `Optional[Union[HandlerResult, 
List[HandlerResult]]]`. For example:
   if no handler then `-> None` else:
   if one statement and `return_last=True` then `-> HandlerResult`
   if `split_statements=True` and `return_last=True` then `-> HandlerResult` 
   if `split_statements=True` and `return_last=False` then `-> 
List[HandlerResult]` 
   
   also for one statement we could return a list with single value
   if one statement and `return_last=False` then `-> List[HandlerResult]`
   
   In code, I assume it'll be
   ```
   if handler is None:
       return None
   elif return_last:
       return results[-1]
   else:
       return results
   ```



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