Reject mismatched rollup mapper and window pairings at Dag parse time#69456
Open
Lee-W wants to merge 1 commit into
Open
Reject mismatched rollup mapper and window pairings at Dag parse time#69456Lee-W wants to merge 1 commit into
Lee-W wants to merge 1 commit into
Conversation
Port the SDK's declared expected_decoded_type onto core partition mappers and compare declarations in both directions when constructing a rollup, so a temporal upstream paired with a segment window fails at parse instead of crashing every scheduler tick. Mappers that override decode_downstream but still decode to str remain valid segment-window upstreams.
Lee-W
requested review from
amoghrajesh,
ashb,
dstandish and
kaxil
as code owners
Lee-W
commented
Jul 6, 2026
|
|
||
| is_rollup: ClassVar[bool] = False | ||
|
|
||
| # Class-level default so the attribute resolves even on subclasses whose |
Member
Author
There was a problem hiding this comment.
This is a missed removal in a previous PR
jason810496
approved these changes
Jul 7, 2026
jason810496
left a comment
jason810496
left a comment
Member
There was a problem hiding this comment.
Thanks for the improvement, LGTM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
RollupMapper's type-pairing guard only caught one direction — a str-decoding mapper against a non-str window — so a datetime mapper paired with a str window slipped through parse and only blew up (silently, no audit log) on a later scheduler tick.What
Both
RollupMappers (core and SDK) now compare the upstream mapper's and window's declaredexpected_decoded_typedirectly, so either direction of mismatch is rejected at Dag parse time.expected_decoded_typebecomes an explicitClassVaron the base mapper (temporal mappers declaredatetime) instead of being inferred from whetherdecode_downstreamis overridden — a mapper that overrides the decode/encode hooks but still decodes tostrstays valid against a str window.Was generative AI tooling used to co-author this PR?
Generated-by: [Claude] following the guidelines
{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.