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

SqlAlchemy InvalidColumnReference error when listing entities that have a lazy="subquery" relationship #210

Closed
2 tasks done
lebrunthibault opened this issue Jun 23, 2022 · 10 comments · Fixed by #212
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@lebrunthibault
Copy link

Checklist

  • The bug is reproducible against the latest release or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

When going on /list the SELECT SQLAlchemy statement fails with the following error :
image

We have an InvalidColumnReference SELECT DISTINCT, ORDER BY expressions must appear in select list

I'm having this error only on a specific model which has a One to One on another table (lazy="subquery")

It seems to be order_by should use the column and not the column name

Steps to reproduce the bug

I'm having Two models, roughtly

class PublicationOrm(Base):
    __tablename__ = "publications"
    id = Column(String, primary_key=True)

    ...

    profile_picture_id = Column(String, ForeignKey("images.id"))
    _cropped_profile_picture = relationship(
        "ImageOrm", lazy="subquery", foreign_keys=[profile_picture_id])


class ImageOrm(Base):
    __tablename__ = "images"

    id = Column(String, primary_key=True)
    _filename = Column("filename", String)
    x = Column(Integer)
    y = Column(Integer)
    width = Column(Integer)
    height = Column(Integer)

I expect someone having this setup and getting the /list page to have a 500 error (see above)

Expected behavior

No error

Actual behavior

500 error

Debugging material

No response

Environment

  • Ubuntu 20
  • python 3.9.5
  • sqlalchemy ==1.4.37

Additional context

No response

@lebrunthibault
Copy link
Author

I've made a pull request if ever I was right : #211

@lebrunthibault
Copy link
Author

lebrunthibault commented Jun 23, 2022

Well it looks like it's not as simple as a one line fix because of the type expected (str) :
column_default_sort: ClassVar[Union[str, tuple, list]] = []

Also assigning a column directly to an admin model propery raises an UnmappedInstanceError from SqlAlchemy because of the descriptor magic. It might not be straightforward to do so, maybe that's why it was done this way ?
e.g.

class UserAdmin(ModelAdmin, model=UserOrm):
    name = 'Utilisateur'
    column_default_sort = UserOrm.email

Could be easier to run the nested select statement without the order by somehow ?

@aminalaee
Copy link
Owner

@lebrunthibault Can you please update the sample code with the SQLAdmin code so that I can reproduce the issue?

Maybe you're talking about two issues at the same time that I could not follow.

And for this:

Also assigning a column directly to an admin model propery raises an UnmappedInstanceError from SqlAlchemy

You can do:

class UserAdmin(ModelAdmin, model=UserOrm):
    name = 'Utilisateur'
    column_default_sort = [(UserOrm.email, False)]  # Or (UserOrm.email, False)

TBH I think in column_default_sort: ClassVar[Union[str, tuple, list]] = [] the list is the best API and then the tuple, the str would be my last option but I just kept it for feature parity for Flask-Admin.

@lebrunthibault
Copy link
Author

lebrunthibault commented Jun 24, 2022

of course. here you go :

# publications.py
from kessel.models.images import ImageOrm


class PublicationOrm(Base):
    __tablename__ = "publications"
    id = Column(String, primary_key=True)

    ...

    profile_picture_id = Column(String, ForeignKey("images.id"))
    _cropped_profile_picture = relationship(
        "ImageOrm", lazy="subquery", foreign_keys=[profile_picture_id])

# images.py
class ImageOrm(Base):
    __tablename__ = "images"

    id = Column(String, primary_key=True)
    _filename = Column("filename", String)
    x = Column(Integer)
    y = Column(Integer)
    width = Column(Integer)
    height = Column(Integer)

# admin/publication.py
from sqladmin import ModelAdmin

from kessel.models.publications import PublicationOrm

class PublicationAdmin(ModelAdmin, model=PublicationOrm):
    name = 'Publication'

And then go to : /publication-orm/list

NB : if I remove lazy="subquery" from the relationship, everything works great

the list is the best API

Oh I see the point for Flask-Admin. I agree with you

@aminalaee
Copy link
Owner

I can't seem to reproduce this, what database and driver are you using?

@lebrunthibault
Copy link
Author

Did you try with data ?

INSERT
INTO images(id, filename)
VALUES (1, 'c6918f95-8711-4934-965a-71c7d998ac6c.jpeg');

INSERT INTO publications (id, profile_picture_id) VALUES (1, 1)

I'm using stock postgresql:// with the timescale db extension

@aminalaee aminalaee added bug Something isn't working and removed waiting-for-feedback labels Jun 24, 2022
@aminalaee
Copy link
Owner

@lebrunthibault It would be great if you could also test it with your project, and also you could update the title as it's not really about default sort column, but another issue with lazy subquery.

@lebrunthibault lebrunthibault changed the title _get_default_sort returning string instead of column, incompatible with SqlAchemy lazy="subquery" relationship SqlAlchemy incompatible with SqlAchemy lazy="subquery" relationship Jun 24, 2022
@lebrunthibault lebrunthibault changed the title SqlAlchemy incompatible with SqlAchemy lazy="subquery" relationship SqlAlchemy InvalidColumnReference error when listing entities that have a lazy="subquery" relationship Jun 24, 2022
@lebrunthibault
Copy link
Author

lebrunthibault commented Jun 24, 2022

I'm sorry I don't have much time today but it doesn't seem to fix the problem for me.

Having my model with lazy="subquery" and no column_default_sort still yields a 500 error on the fix-subquery-issue-in-list-page branch

Thanks anyway, I'll be able to test again on monday
Have a good weekend

@aminalaee
Copy link
Owner

@lebrunthibault It does seem to solve the problem for me, can you please provide the what the issue is with this implementation?

@lebrunthibault
Copy link
Author

@aminalaee,

My bad I might have tested the wrong branch it actually works :)
Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants