hkc-8010 opened a new pull request, #71926:
URL: https://github.com/apache/airflow/pull/71926
closes: #71921
## Why
Migration `0049_3_0_0_remove_pickled_data_from_xcom_table` quotes non-finite
floats before the
`bytea -> JSONB` cast:
```sql
regexp_replace(txt, '([:,\[]\s*|^)(NaN|-?Infinity)(?=\s*[,}\]]|$)',
'\1"\2"', 'g')
```
The inserted quotes are unescaped, which is only valid when the token sits
in a top-level JSON
document. When a task pushes already-serialized JSON, the stored value is a
JSON string wrapping a
JSON document and its interior quotes are backslash-escaped, so the
sanitizer rewrites
`\"commission\": NaN}` to `\"commission\": "NaN"}`. The raw quote closes the
wrapping string, the
following `NaN` becomes a bare token, and the cast aborts:
```
sqlalchemy.exc.DataError: (psycopg2.errors.InvalidTextRepresentation)
invalid input syntax for type json
DETAIL: Token "NaN" is invalid.
CONTEXT: JSON data, line 1: "{\"amount\": 604441.0, \"commission\": "NaN...
[SQL: ALTER TABLE xcom ALTER COLUMN value TYPE JSONB USING ...]
```
The sanitizer is what creates the invalid JSON. The stored bytea is exactly
what Airflow 2's
`json.dumps` writes for a NaN float and it casts to JSONB fine before
sanitization.
This is not fixed in any release. #53812, #57614, #57866 and #62686 each
refined which tokens get
matched, and #69064 added the NUL and invalid-byte guards, but all of them
insert unescaped quotes.
On the deployment that prompted this, the migration job looped 144 times
over 35 hours with no
scheduler or API server up, and the only way out was hand-editing rows in
the metadata DB.
## What changed
All three dialect branches now substitute the bare literal `null` instead of
a quoted string. The
replacement has to be valid at whatever nesting depth the token occupies and
a quote character is
not, while `null` needs no escaping and is correct at every depth.
I did look at keeping the value. Deciding the escaping depth needs to know
whether the enclosing
string is escaped, and that is not available locally to the match: for the
`key: NaN` form you could
look behind at the key's closing quote, but array elements (`[NaN,
Infinity]`) have no adjacent
quote, and a lookahead for the next escaped quote breaks when the token is
the last one before the
wrapping string closes, which is the shape in the linked issue. A heuristic
that is wrong on some
inputs seemed like a bad trade in a one-shot irreversible migration, so this
takes the depth-agnostic
route instead.
The MySQL branch had a second problem that the new tests exposed. Its
pattern put the run of
spaces after the delimiter outside group 1, so the replacement dropped it and
`\"commission\": NaN` came back as `\"commission\":null`. PostgreSQL already
keeps that whitespace
inside group 1. It made no difference in a top-level document since JSON
ignores whitespace, but
when the document is itself a JSON string those bytes are the value the
consumer reads back, so
this moves the `[ ]*` inside the group to match PostgreSQL.
The SQLite branch gets simpler as a side effect. Since all three tokens map
to `null`, the
`-Infinity` -> `Infinity` -> `NaN` order replaces the previous
quote-then-repair-the-`-"Infinity"`-artifact
sequence.
Migration 0055 (`dag_run.conf`) is not affected and is unchanged apart from
a comment. It converts
the deserialized Python object and lets `json.dumps` do the quoting, which
gets the escaping right at
any depth. The old comment claimed it mirrored 0049; that is no longer true
and the reason is worth
recording.
### Behaviour change
A non-finite float in a top-level XCom document previously became the string
`"NaN"` / `"Infinity"` /
`"-Infinity"` and now becomes `null`. This only affects deployments that
have not yet run the 2.x ->
3.x upgrade.
## Tests
`test_0049_remove_pickled_data_from_xcom_table.py` gains two rows in each of
the three dialect tests:
the whole XCom value being the serialized document, and the serialized
document sitting under a key in
a dict. The existing rows keep covering the top-level tokens, the embedded
NUL escape, and the
literal backslash-u-0000 that must survive untouched.
The SQLite test fails on current `main` with `sqlite3.OperationalError:
malformed JSON` and passes
here.
I also ran the real `_xcom_pg_sanitize_sql` and `_xcom_mysql_sanitize_sql`
against PostgreSQL 16 and
MySQL 8.4 directly. On `main` both reproduce the failure, PostgreSQL with
the production
`CONTEXT: ... \"commission\": "NaN...` message and MySQL with
`Invalid JSON text ... The document root must not be followed by other
values.`. Both pass with this
change, including an array of non-finite tokens inside an escaped string and
a bare scalar `NaN`.
--
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]
