[GitHub] [airflow] eladkal commented on a change in pull request #22391: Update the redshift sql operator to accept multiple sql statements

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

eladkal commented on a change in pull request #22391:
URL: https://github.com/apache/airflow/pull/22391#discussion_r830917171



##########
File path: airflow/providers/amazon/aws/operators/redshift_sql.py
##########
@@ -74,5 +75,12 @@ def get_hook(self) -> RedshiftSQLHook:
     def execute(self, context: 'Context') -> None:
         """Execute a statement against Amazon Redshift"""
         self.log.info(f"Executing statement: {self.sql}")
+        sql_stmts = sqlparse.split(self.sql)
         hook = self.get_hook()
-        hook.run(self.sql, autocommit=self.autocommit, parameters=self.parameters)
+
+        with hook.get_conn() as con:
+            con.autocommit = self.autocommit
+            with con.cursor() as cursor:
+                for stmt in sql_stmts:
+                    cursor.execute(stmt)
+                    if self.autocommit is False: con.commit()

Review comment:
       I don't think this is the right approach.
   The functionality should exist in the hook and the operator should utilize it.
   The operator should not use cursor directly as you do it here.
   
   See https://github.com/apache/airflow/pull/15533 adding similar functionality for Snowflake




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