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

Not .select()'ed foreign key field raises DoesNotExist with lazy_load=False #2377

Closed
AndBondStyle opened this issue Mar 20, 2021 · 3 comments

Comments

@AndBondStyle
Copy link

I have a model with a foreign key field like this:

related_id = ForeignKeyField(OtherModel, lazy_load=False)

If you don't include this field in a select query and then try to access it, you get DoesNotExist error:

x = Model.select(Model.id).get()  # related_id field not included
print(x.related_id)  # raises DoesNotExist

... which is not very expected, considering lazy_load=False.

The root of exception is in the ForeignKeyAccessor.get_rel_instance method, and I believe it should be changed like this:

def get_rel_instance(self, instance):
    value = instance.__data__.get(self.name)
    if value is not None or self.name in instance.__rel__:
        if self.name not in instance.__rel__ and self.field.lazy_load:
            obj = self.rel_model.get(self.field.rel_field == value)
            instance.__rel__[self.name] = obj
        return instance.__rel__.get(self.name, value)
-    elif not self.field.null:
+    elif not self.field.null and self.field.lazy_load:
        raise self.rel_model.DoesNotExist
    return value
@coleifer
Copy link
Owner

Looks like a regression related to #2328

@coleifer
Copy link
Owner

Actually scratch that - this is not a regression. There doesn't seem to be a reliable way of differentiating between a model instance you have explicitly chosen not to select the foreign-key value and a model instance that is missing a non-null foreign-key. Lazy-load really deals with converting a foreign-key value (e.g. integer id) into a full object.

@coleifer
Copy link
Owner

That being said, I agree that the error is misplaced and this should just return None if lazy-load has been disabled. I've made the patch as you suggested.

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

No branches or pull requests

2 participants