[GitHub] [airflow] kazanzhy commented on a diff in pull request #25412: Fixing JdbcOperator non-SELECT statement run

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

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


##########
airflow/providers/jdbc/operators/jdbc.py:
##########
@@ -57,16 +57,21 @@ def __init__(
         jdbc_conn_id: str = 'jdbc_default',
         autocommit: bool = False,
         parameters: Optional[Union[Iterable, Mapping]] = None,
+        fetch_results: bool = False,
         **kwargs,
     ) -> None:
         super().__init__(**kwargs)
         self.parameters = parameters
         self.sql = sql
         self.jdbc_conn_id = jdbc_conn_id
         self.autocommit = autocommit
+        self.fetch_results = fetch_results
         self.hook = None
 
     def execute(self, context: 'Context'):
         self.log.info('Executing: %s', self.sql)
         hook = JdbcHook(jdbc_conn_id=self.jdbc_conn_id)
-        return hook.run(self.sql, self.autocommit, parameters=self.parameters, handler=fetch_all_handler)
+        if self.fetch_results:

Review Comment:
   No, you're right. It's I missed that no additional parameter needed if `do_xcom_push` is already present



##########
airflow/providers/jdbc/operators/jdbc.py:
##########
@@ -57,16 +57,21 @@ def __init__(
         jdbc_conn_id: str = 'jdbc_default',
         autocommit: bool = False,
         parameters: Optional[Union[Iterable, Mapping]] = None,
+        fetch_results: bool = False,
         **kwargs,
     ) -> None:
         super().__init__(**kwargs)
         self.parameters = parameters
         self.sql = sql
         self.jdbc_conn_id = jdbc_conn_id
         self.autocommit = autocommit
+        self.fetch_results = fetch_results
         self.hook = None
 
     def execute(self, context: 'Context'):
         self.log.info('Executing: %s', self.sql)
         hook = JdbcHook(jdbc_conn_id=self.jdbc_conn_id)
-        return hook.run(self.sql, self.autocommit, parameters=self.parameters, handler=fetch_all_handler)
+        if self.fetch_results:

Review Comment:
   No, you're right. I missed that no additional parameter is needed if `do_xcom_push` is already present



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