hussein-awala commented on PR #29599:
URL: https://github.com/apache/airflow/pull/29599#issuecomment-1435737748
> @hussein-awala - the bug _is_ specifically in the SnowflakeProvider - the
base class just does a no-op with `_process_output` and isn't affected, I was
hesitant to add it there in case the fix interferes with other providers some
how by skipping a `_process_output` that does do something with an empty result
set thinking
>
> ```
> --- a/airflow/providers/common/sql/operators/sql.py
> +++ b/airflow/providers/common/sql/operators/sql.py
> @@ -265,6 +265,9 @@ class SQLExecuteQueryOperator(BaseSQLOperator):
> return_last=self.return_last,
> **extra_kwargs,
> )
> + # Handle do_xcom_push=False
> + if output is None or output == [None]:
> + return []
> if return_single_query_results(self.sql, self.return_last,
self.split_statements):
> # For simplicity, we pass always list as input to
_process_output, regardless if
> # single query results are going to be returned, and we
return the first element
>
> --- a/airflow/providers/snowflake/operators/snowflake.py
> +++ b/airflow/providers/snowflake/operators/snowflake.py
> @@ -108,9 +108,6 @@ class SnowflakeOperator(SQLExecuteQueryOperator):
> results: Optional[list[Any]],
> descriptions: list[Sequence[Sequence] | None]
> ) -> list[Any]:
> - # Handle do_xcom_push=False
> - if results is None or results == [None]:
> - return []
> validated_descriptions: list[Sequence[Sequence]] = []
> for idx, description in enumerate(descriptions):
> if not description:
> ```
I think adding a condition on the `SQLExecuteQueryOperator` without changing
the snowflake operator is enough to solve the problem, since we don't need to
process the result because we don't want to push it as a xcom:
```diff
--- a/airflow/providers/common/sql/operators/sql.py
+++ b/airflow/providers/common/sql/operators/sql.py
@@ -265,6 +265,8 @@ class SQLExecuteQueryOperator(BaseSQLOperator):
return_last=self.return_last,
**extra_kwargs,
)
+ if not self.do_xcom_push:
+ return None
if return_single_query_results(self.sql, self.return_last,
self.split_statements):
# For simplicity, we pass always list as input to
_process_output, regardless if
# single query results are going to be returned, and we return
the first element
```
What do you think?
--
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]
