[GitHub] [airflow] potiuk commented on a change in pull request #16676: Sql query results with snowflake operator

[GitBox <gi...@apache.org>]

potiuk commented on a change in pull request #16676:
URL: https://github.com/apache/airflow/pull/16676#discussion_r659192505



##########
File path: airflow/hooks/dbapi.py
##########
@@ -211,8 +211,13 @@ def _run_command(self, cur, sql_statement, parameters):
         self.log.info("Running statement: %s, parameters: %s", sql_statement, parameters)
         if parameters:
             cur.execute(sql_statement, parameters)
+            res = cur.fetchall()
+            result={'sql_statement': res,'headers': list(map(lambda t: t[0], cur.description))}
+            self.log.info("Query Results: %s", result)

Review comment:
       0This will actually impact ALL DB operators - not only Snowflake. I think info level here is quite a bit too much - possibly Debug will be better  - also running fetchall is potentially impacting the performance and seems we only need it to print logging information.
   
   I'd change it to debug level and put `fetchall` under:
   
   ```
   if self.log.isEnabledFor(logging.DEBUG):
   ```
   
   to minimise side-effects
   




-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org