Repository navigation
Fix DagRun.conf exponent-form float handling in PostgreSQL JSONB - #74093
kadubhumika wants to merge 4 commits into
Conversation
|
@eladkal Hi! Quick update on the PR I’ve addressed the two CI issues that came up: Fixed the MySQL Core test failure by making the regression test compatible with MySQL’s handling of very large integers. |
| return super().load_dialect_impl(dialect) | ||
|
|
||
|
|
||
| class DagRunConfJSON(TypeDecorator): |
There was a problem hiding this comment.
Do we need to this? Is there not an option to instead make anything larger than 64bit it force to float before we send to msgspec.
In other words, this isn't a storage or JSON vs JSONB issue, it's an encoding issue.
There was a problem hiding this comment.
Hi @ashb , I checked the downstream behavior.
1.7e308 can become a 309-digit Python int after the PostgreSQL JSONB round-trip, and msgspec.msgpack.encode() raises OverflowError for that value.
I also found that #73783 addresses the MessagePack overflow for large integers, but it does not restore the original float type after the JSONB round-trip.
I’ll now trace the Airflow encoding path to see whether converting oversized integers to floats before MessagePack is sufficient and safe, rather than changing the JSONB storage layer.
There was a problem hiding this comment.
I need one day i will solve fix inspect this part thoroughly.
There was a problem hiding this comment.
Thanks @ashb Sir, I traced the full path through _FrameMixin.as_bytes()
and also inspected #73783.
I see that #73783 already addresses the MessagePack OverflowError
by preserving oversized integers through a MessagePack extension.
The remaining difference is the separate float → int conversion caused
by the PostgreSQL JSONB round trip. For example, 1.7E308 is a float
before persistence but becomes a 309-digit int after reading it back.
So I understand now that the encoding crash and the numeric-type
preservation are separate issues. Before changing the implementation,
I'd like to confirm whether Airflow 3 is expected to preserve the
original float type here, or whether #73783's exact-int behavior is
the intended semantics. Sorry Next time before working on any issues or prs i will first t inspect all existing's prs on it.. Thank you .. I learned a lott..
There was a problem hiding this comment.
@ashb how would you like to proceed? Are you closing this PR for now, or is there a specific way you'd like me to update it ?
I'm completely happy to follow your lead.
Summary
PostgreSQL JSONB can change large exponent-form floating-point values in
DagRun.confinto large integers when Airflow reads them back.For example,
1.7E308can be returned as a Pythonintinstead of afloat.This PR:
DagRun.confto preserve exponent-form floats.floatwhen reading them back.Testing
airflow-core/tests/unit/models/test_dagrun.py: 248 passed, 1 expected xfailedgit diff --check: passed1.7E308is stored by PostgreSQL JSONB as a large integer-like numeric representation.jsonb_typeof(...)reports the value asnumber.airflow_sharedimport issue was resolved by installing the repository'sshared/secrets_maskerpackage in the development environment.The SQLAlchemy → PostgreSQL end-to-end round-trip test is currently limited by the local PostgreSQL
pg_hba.confconfiguration for WSL connections.Reproduced the PostgreSQL JSONB behavior independently with PostgreSQL 18.3: 1.7E308 is stored as a large integer-like numeric value.
Added regression coverage for DagRun.conf and verified the exponent-form value is restored as a Python float.
Ran the complete airflow-core/tests/unit/models/test_dagrun.py: 248 passed, 1 expected xfailed.
Ruff and git diff --check both pass.
Resolved the earlier airflow_shared local environment issue and reran the affected tests successfully.
I also verified the PostgreSQL JSONB behavior independently. The remaining SQLAlchemy → PostgreSQL round-trip test is currently blocked by my local PostgreSQL pg_hba.conf configuration for WSL connections, so I have not marked that integration test as passing.
Closes #74023