Optimize joins in search and sorting, add related tests#903
Optimize joins in search and sorting, add related tests#903pokidovea-playrix wants to merge 3 commits intosmithyhq:mainfrom
Conversation
Implemented logic to avoid redundant table joins in search and sorting by tracking joined tables. Added tests to validate searching by multiple related columns to ensure correctness and robustness. Updated `.gitignore` to include `.idea/`.
Replaced direct `__tablename__` access with `class_mapper(model).mapped_table.name` for better compatibility and reliability.
|
|
||
| @staticmethod | ||
| def _get_joined_tables(stmt: Select) -> set[str]: | ||
| return {table.name for table, *_ in stmt._setup_joins} |
There was a problem hiding this comment.
Hi @pokidovea-playrix , thanks for your contribution.
I think stmt._setup_joins is not recommended because it is a private (internal) attribute of SQLAlchemy. Code may break if SQLAlchemy changes this in the future.
| return pairs | ||
|
|
||
| @staticmethod | ||
| def _get_joined_tables(stmt: Select) -> set[str]: |
There was a problem hiding this comment.
As the package should be compatible with Python 3.8, please use Set[str] instead of set[str].
| ) | ||
|
|
||
|
|
||
| class Table3(Base): |
There was a problem hiding this comment.
Model names could be more descriptive and intuitive, clearly reflecting a real-world relations. For example : Continent, Country, City
| def base_content(): | ||
| with SessionLocal() as session: | ||
| # Add base data to the database | ||
| t1 = Table1(id=1, field1="Field1_Value1") |
There was a problem hiding this comment.
Variable naming can be more descriptive
| yield c | ||
|
|
||
|
|
||
| def base_content(): |
There was a problem hiding this comment.
Isn't better to make it a fixture?
| ) -> None: | ||
| base_content() | ||
| response = client.get(f"/admin/table3/list?search={term}") | ||
| assert response.status_code == 200 |
There was a problem hiding this comment.
It would be nice if test follows Arrange-Act-Assert pattern.
mmzeynalli
added
the
waiting-for-feedback
|
@pokidovea any updates? This is the same PR as #983. I have to merge one of these, I will go with the one that is approved first. Will close second due to duplication. |
5 participants
Implemented logic to avoid redundant table joins in search and sorting by tracking joined tables. Added tests to validate searching by multiple related columns to ensure correctness and robustness. Updated
.gitignoreto include.idea/.Related issue #901