Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure add_stator_indexes only run for direct descendants of StatorModel #696

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lullis
Copy link
Contributor

@lullis lullis commented Jan 30, 2024

No description provided.

@AstraLuma
Copy link
Contributor

Does this have a bug associated with it? What's the behavior you're trying to prevent?

@lullis
Copy link
Contributor Author

lullis commented Feb 6, 2024

The issue is that these indices only make sense on models that derive directly from StatorModel. If you create any other model that itself is already derived from StatorModel (e.g, Identity) then the system will fail the check due to issues with the indices that are not related to the table you are creating.

@AstraLuma
Copy link
Contributor

Ah, ok, yeah, if you have a concrete model inheriting from a concrete model (Multi Table Inheritance), this code will fail because the indexed columns only exist on the parent table.

This largely looks good, but I'm going to double check some things.

"""
if issubclass(sender, StatorModel):
if sender.__base__ is StatorModel:
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 completely thrilled with this--__base__ is semi-documented, and points to the first parent class.

But I'm not sure what multiple inheritance would do in the context of Django models.

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