[GitHub] [airflow] kazanzhy commented on a diff in pull request #25717: Add SQLExecuteQueryOperator

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

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


##########
airflow/providers/common/sql/operators/sql.py:
##########
@@ -162,6 +164,68 @@ def get_db_hook(self) -> DbApiHook:
         return self._hook
 
 
+class SQLExecuteQueryOperator(BaseSQLOperator):
+    """
+    Executes SQL code in a specific database
+    :param sql: the SQL code or string pointing to a template file to be executed (templated).
+    File must have a '.sql' extensions.
+    :param autocommit: (optional) if True, each command is automatically committed (default: False).
+    :param parameters: (optional) the parameters to render the SQL query with.
+    :param handler: (optional) the function that will be applied to the cursor.
+    :param split_statements: (optional) if split single SQL string into statements (default: False).
+    :param return_last: (optional) if return the result of only last statement (default: True).
+    """
+
+    template_fields: Sequence[str] = ('sql', 'parameters')
+    template_ext: Sequence[str] = ('.sql',)
+    ui_color = '#cdaaed'
+
+    def __init__(
+        self,
+        *,
+        sql: str | list[str],
+        autocommit: bool = False,
+        parameters: Mapping | Iterable | None = None,
+        handler: Callable[[Any], Any] = fetch_all_handler,
+        split_statements: bool = False,
+        return_last: bool = True,

Review Comment:
   Based on my previous PR which had broke compatibility I decided to avoid it here.
   
   Talking about the `return_last` it works only when `do_xcom_push=True` (by default True) and `split_statements=True` (by default False).
   Probably it should be renamed to `return_last_if_split_statements` :)
   Why it's `True` by default?
   I think when we added `split_statements` we wanted to keep the same returning type independent what were passed there.
   https://github.com/apache/airflow/pull/23971#discussion_r922847638
   
   If the user has in the legacy code, let's say, PostgresOperator. By default 
   it has `do_xcom_push = True`, `split_statements = False` and `return_last = True`. 
   https://github.com/apache/airflow/blob/69da98cdb194de3544368b6bd7c47dcc7ace8814/airflow/models/baseoperator.py#L735
   https://github.com/apache/airflow/blob/69da98cdb194de3544368b6bd7c47dcc7ace8814/airflow/providers/common/sql/hooks/sql.py#L261
   
   So if the's a string as an SQL parameter. then result of this query will be returned. If the list of strings is passed as an SQL parameter, then wil be returned list of results of each statement. Of course in case if handler were passed. I think here we discussed it here https://github.com/apache/airflow/pull/23971#discussion_r885115775
   
   
   
   



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