Skip to content

Performance/n+1 query ical export#2575

Open
Tschuppi81 wants to merge 6 commits into
masterfrom
performance/n+1-query-ical-export
Open

Performance/n+1 query ical export#2575
Tschuppi81 wants to merge 6 commits into
masterfrom
performance/n+1-query-ical-export

Conversation

@Tschuppi81

@Tschuppi81 Tschuppi81 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Event: Fixes the N+1 query by dropping eagerly loading on the unused relationships in the iCal export

TYPE: Performance
LINK: https://seantis-gmbh.sentry.io/issues/7605741267

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.49%. Comparing base (2794281) to head (8131582).
✅ All tests successful. No failed tests found.

Additional details and impacted files
Files with missing lines Coverage Δ
src/onegov/event/collections/occurrences.py 97.40% <100.00%> (+0.01%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2794281...8131582. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Daverball

Copy link
Copy Markdown
Member

Technically a false positive, since it's more like a N/500 + 1 query, it appears that SQLAlchemy loads the related objects in batches of 500 for the selectin strategy.

But since we actually don't need the occurrences here, it doesn't hurt to skip loading them. But I would probably switch to noload or raiseload from lazyload though, so we don't get a real N+1 query in the future, if we do end up accessing the occurrences for something. We'd rather know about this performance regression early, by virtue of our tests failing.

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