Skip to content

Fix: Direct breaking category should only impact direct children snapshot#3975

Merged
izeigerman merged 2 commits intomainfrom
fix-breaking-changes-only-impact-immediate-children
Mar 12, 2025
Merged

Fix: Direct breaking category should only impact direct children snapshot#3975
izeigerman merged 2 commits intomainfrom
fix-breaking-changes-only-impact-immediate-children

Conversation

@izeigerman
Copy link
Collaborator

In a DAG that looks like A -> B -> C, if the user makes a Breaking change to A and a Non-breaking change to B, then C should be categorized as Indirect Non-Breaking rather than Indirect Breaking, as it was before this change.

At the same time, if A was categorized as Forward-Only and B as Non-Breaking, SQLMesh should prefer the Forward-only category for C over the Indirect Non-Breaking one.

@izeigerman izeigerman requested a review from a team March 11, 2025 22:32
direct_parent_categories.append(parent.change_category)

if not categories or any(
if not direct_parent_categories or any(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there are no direct parent categories, but then grandparent A is forward only, is this still correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can there be no direct parent categories?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: it's direct parent not direct category

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, let me rephrase the question. How can there be a grandparent category but not a parent category?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i’m not sure, is there a case of a revert or something where it’s possible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, I'm having a hard time coming up with a scenario like this. A scenario like this would indicate that there's an orphaned uncategorized new snapshot which is somehow indirectly modified. I think falling back to breaking in this case seems reasonable.

@izeigerman izeigerman merged commit 8c793ab into main Mar 12, 2025
22 checks passed
@izeigerman izeigerman deleted the fix-breaking-changes-only-impact-immediate-children branch March 12, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants