Correctly pre-allocate external_executor_id. with multiple executors.#67388
Correctly pre-allocate external_executor_id. with multiple executors.#67388ashb wants to merge 1 commit into
external_executor_id. with multiple executors.#67388Conversation
This wasn't working on PostgreSQL when you used CeleryExecutor with another executor as the arms of the CASE had different types: ``` sqlalchemy.exc.ProgrammingError: (psycopg2.errors.DatatypeMismatch) CASE types text and uuid cannot be matched ``` The fix is to use an explicit cast.
external_exeuctor_id with multiple executors.external_executor_id. with multiple executors.
jscheffl
left a comment
There was a problem hiding this comment.
Pure code-reading looks good. Unfortunately I am OOO but carry my business laptop with me and attempt to repro with this patched... but can not promise tomorrow. But code looks as promising that I assume "looks good" w/o testing in our env.
| @@ -953,9 +955,9 @@ def _executable_task_instances_to_queued(self, max_tis: int, session: Session) - | |||
| opt_in_names.add(exc.name.module_path) | |||
| whens = [] | |||
| if opt_in_names: | |||
There was a problem hiding this comment.
Is it worth baking the cast into random_db_uuid itself instead of wrapping every call site? In airflow-core/src/airflow/utils/sqlalchemy.py:
@compiles(random_db_uuid, "postgresql")
def _random_db_uuid_pg(element, compiler, **kw):
return "gen_random_uuid()::text"That makes the function safe in any expression context (CASE, UNION, CTE, INSERT...SELECT, RETURNING) without each caller having to remember sql_cast(..., Text). The unconditional path at L944 only works today because PG has an implicit assignment cast (SET text_col = uuid_expr); a future call site in a stricter context would hit the same DatatypeMismatch and not be obvious why.
Not blocking!
This wasn't working on PostgreSQL when you used CeleryExecutor with another
executor as the arms of the CASE had different types:
The fix is to use an explicit cast. Reported by @jscheffl in slack and in #67385
closes: #67385
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.