Skip to content

Migrate to SQLModel and improve relationship loading#4547

Merged
kitsuta merged 20 commits intomainfrom
load-relationships-on-purpose
Mar 11, 2026
Merged

Migrate to SQLModel and improve relationship loading#4547
kitsuta merged 20 commits intomainfrom
load-relationships-on-purpose

Conversation

@kitsuta
Copy link
Copy Markdown
Member

@kitsuta kitsuta commented Feb 6, 2026

It turned out that upgrading SQLAlchemy, which made all our model properties None before saving to the database, broke our form instantiating logic. Rather than fight with WTForms to accommodate that, I chose to move us to SQLModel to let us set Pydantic default types on models defined for SQLAlchemy. This involved:

  • Replacing all backref relationship definitions, which were deprecated in SQLAlchemy and unsupported in SQLModel, with explicit relationships
  • Reworking our cascade deletes:
    • We were setting the default cascade to delete but only on relationships not defined by backref, which meant that half of our relationships were NOT doing cascade deletes by default. Now that all relationships are explicit, the better default is save-update/merge/etc, with cascade deletes defined manually
    • The SQLAlchemy docs recommend configuring cascade deletes at the foreign key level for the DB and at the SQLAlchemy level, but we weren't doing that in most cases. Also added the passive_delete option to support those DB-level cascade deletes
  • The uselist property was removed as it's no longer required -- SQLAlchemy is smart enough to see whether we type the relationship property as list[Model] or just a singular Model
  • The SQLAlchemy docs recommend pairing single_parent with a unique constraint to enforce one-to-one relationships. The single_parent property has also been removed from any relationship that isn't one-to-one

Separately, an attempt was made to default our relationships to raise loading to prevent excessive lazy loads/N+1 queries. This ultimately didn't pan out because of the enormous scope involved, but I did review all current relationships for places where it was appropriate to do joined loading or selectin loading by default, so we should still see some improvements.

Old description below since it has some to-dos that are worth keeping:

Changes the default relationship loading strategy to raise in order to avoid generating N+1 queries.

We override this default on many relationships at the mapper level to save on needing to do it at the query level. For one, most child objects in many-to-one relationships do one-sided joined eager loading for their parent object. There are some exceptions based on how “important” the parent is to the children (so, e.g., Attendee and Group creators don’t get joined automatically because we rarely use that relationship). Additionally, tables that are very closely tied together or frequently reference each other get joined or selectin loading on both sides of their relationships (joined if it’s a one-to-one or many-to-one, selectin if not).

TODO:

  • There’s a lot of relationships, especially surrounding departments/jobs, that are effectively used as query shortcuts. I do not like that and would like to remove them.
  • Only a few pages work currently and many queries need to be updated, especially on the Attendee object where we leave raise on most child relationships.
  • Related to the above point, I slapped select (which is what we think of as 'lazy loading') on a few relationships just to get things to load. Most of those should be removed, except in a few cases where we're sure it's unlikely to cause problems (e.g., there's no reason to access the PasswordReset relationships in bulk, so it's probably fine to leave it).
  • Need to decide what to do with the active_receipt relationship. This relationship IS used frequently (especially on the Attendee object, where we're constantly checking their amount unpaid), but the ModelReceipt, ReceiptTransaction, and ReceiptItem tables are all joined together (for good reason) so eager loading them all every time we touch the attendee seems inappropriate. OTOH, we are probably doing that anyway so it might be best to just add joined to the mapper, at least for now.
  • Once we get most queries/most pages working we'll have a good idea of what kinds of queries are the most common and we can pull those out into services, which will be a good opportunity to reorganize the backend file structure.

Changes the default relationship loading strategy to raise in order to avoid generating N+1 queries. We also override this default on many relationships based on how we use those relationships.

You can currently load and save the attendee form and load the attendee shifts page.
@kitsuta kitsuta marked this pull request as draft February 6, 2026 12:58
A lot of paginated pages would break if they had 0 rows to display. This fixes that.
Noticed this while testing the API reference page -- several services didn't have prefilled suggestions.
We still want the old behavior for repr as that makes the Tracking table much easier to read. Also fixes issues with passing fields to to_dict.
Adds required loading to pages accessible from the site map. Not all reports were checked, and forms are not working right now so testing was quite basic (e.g. most pages had no data to display).

Also adds relationship hints to each model because I got tired of jumping around different relationship definitions to remember which relationships were set to be joined/selected in by default.
Adds the SQLModel library and updates all the model definitions to define their type. However, indexes seem to be broken still.
We had to remove backrefs as they broke SQLModel's ability to assign the backref columns. Also recasts all the foreign keys into SQLModel's Field(), and partially converts the regular columns (for Attendee and Department, specifically). Thanks to all this and a couple other fixes, forms will load and save now.
Adds a new version of the relationship and column overrides to account for the use of Field() and Relationship() on their own, and updates some types of fields to use sa_type instead of sa_column in order to get to server to stop generating the wrong kind of column in migrations. Also removes datetime.utcnow() as it's deprecated.
Fixes FK key name generation and the corresponding migration, and finishes converting all columns to SQLModel.
A bit of a clumsy wrap-up, but this rolls back the raise relationship loading strategy while also fixing bugs introduced by the SQLAlchemy upgrade/SQLModel implementation.
@kitsuta kitsuta changed the title Overhaul relationship loading strategies Add SQLModel and improve relationship loading Feb 25, 2026
@kitsuta kitsuta changed the title Add SQLModel and improve relationship loading Migrate to SQLModel and improve relationship loading Feb 25, 2026
@kitsuta kitsuta marked this pull request as ready for review February 25, 2026 13:49
Takes care of a few bugs when you edit a badge during prereg.
with Session() as session:
initialize_db()
session = Session().session No newline at end of file
session = Session() No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this need to be called? Shouldn't session already be defined and set to the session by the context manager?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Huh, yea, this must have been an old copy-paste error.

kitsuta added 6 commits March 8, 2026 14:02
Anytime you set a novel attribute that started with 'is_', we were checking to see if the object's model class matched what came after. I didn't even know our code did this and we only use it in one place inside the magprime plugin, so we're removing it now.

Also removes a 'return' on __setattr__.
Fixes a find-and-replace issue that was breaking emails.
In cases where we were defining the ribbon as part of the object, it was ending up as an integer and not a string. Hopefully ribbons are the only Choice column that we sometimes set manually to an integer (which we probably shouldn't, but do due to legacy code where you could only have one ribbon at a time).
We weren't adding columns as columns anymore. This is only a partial solution as it's not 1:1 -- you have to define all columns in plugins as Field() variables -- but it's working at least.

Also makes our migrations use sa.Unicode instead of sqlmodel's unnecessary AutoString class.
This breaks our ability to correctly generate migrations.
@kitsuta kitsuta merged commit 2031885 into main Mar 11, 2026
3 checks passed
@kitsuta kitsuta deleted the load-relationships-on-purpose branch March 11, 2026 21:02
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