Skip to content

Conversation

@izeigerman
Copy link
Member

No description provided.

@izeigerman izeigerman requested review from a team and Copilot October 30, 2025 20:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds caching for upstream dependency resolution in the scheduler to optimize performance when handling complex diamond-shaped dependency graphs. The caching prevents redundant traversals of transitive dependencies when multiple paths lead to the same upstream snapshot.

Key Changes

  • Modified _find_upstream_dependencies to use a cache that memoizes upstream dependencies for each SnapshotId
  • Changed return types from List[SchedulingUnit] to Set[SchedulingUnit] to prevent duplicates and maintain uniqueness
  • Added a new test case covering a complex diamond dependency pattern with multiple convergent paths

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
sqlmesh/core/scheduler.py Added upstream dependency caching mechanism and changed collection types from lists to sets for deduplication
tests/core/test_scheduler.py Added test case validating correct handling of complex diamond dependency graphs with caching

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@izeigerman izeigerman merged commit ac5446b into main Oct 30, 2025
35 of 36 checks passed
@izeigerman izeigerman deleted the fix-cache-upstream-when-building-eval-dag branch October 30, 2025 21:21
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.

4 participants