Add support to declarative joins [SQLAlchemy]

[agusmdev]

Just added a few lines of code to allow the "declarative" join definition. Another user created a similar PR but doing "magic" or "smart" joins, but I find the declarative approach clearer. For the test cases I just removed the manual joins in the select query and added the declarative approach

[netlify]

✅ Deploy Preview for fastapi-filter ready!

Name	Link
🔨 Latest commit	c01ef72
🔍 Latest deploy log	https://app.netlify.com/sites/fastapi-filter/deploys/66466e42569c47000866220a
😎 Deploy Preview	https://deploy-preview-585--fastapi-filter.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (97d5767) to head (c01ef72).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #585   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          190       197    +7     
=========================================
+ Hits           190       197    +7     

Files	Coverage Δ
fastapi_filter/contrib/sqlalchemy/filter.py	100.00% <100.00%> (ø)

[arthurio]

Thank you for your contribution Agus!

I very much prefer this declarative approach indeed. There is one caveat where people might shoot themselves in the foot, if they already have the join define on their base query (let say that they nest the related object in their response's object), it will throw sqlalchemy.exc.OperationalError "ambiguous column name" because the join is applied twice.

1. Would it be possible to detect that the join is already present and skip it if so? If not, then people need to know that they have to decide which one they use, with the caveat that the filter join is applied only if there is a filter on that related field.
2. One nit picky thing is that I don't love the join_kwargs name, maybe just joins (since you can have multiple)?
3. I also like that this is backward compatible but we should definitely add something in the docs (yeah I know they could use some love...).
4. Finally, can we add a link to https://docs.sqlalchemy.org/en/20/core/selectable.html#sqlalchemy.sql.expression.Select.join so that people know what parameters are allowed in the dict?

[arthurio]

So here we can just omit the target?

[agusmdev]

Yep! exactly :D, actually most of the time it would be enough to do something like this:
"address": {}

[agusmdev]

Thank you for your contribution Agus!

Thank you for doing such a useful package!

1. Would it be possible to detect that the join is already present and skip it if so? If not, then people need to know that they have to decide which one they use, with the caveat that the filter join is applied only if there is a filter on that related field.

I was digging a bit into this topic, but I think I'd leave it as it is now: if the user joins twice then there will be an error in the query execution later. I tried to come up with an elegant "smart" solution, and I found that SQLAlchemy saves the tables will be joined in an attribute called _setup_joins and there you can just check if the table you're trying to join is already present or not, but this gets a bit more complicated in non-trivial cases, because if you use models that are aliased then the object is not a Table, it's an Alias, so you don't have the same interface and you have to write custom code for each type of valid object to join. This is something I know because I have a use case where I must use an Alias to join the same table twice in different attributes but I don't know any other "advanced" use cases that should also be supported for this smart logic.

just FYI:

I was exploring this code for checking:

    @staticmethod
    def is_table_joined(table_name: str, query: Select) -> bool:
        """Check if a table is already joined in a query by checking the setup join element.

        It's of type: Tuple[_JoinTargetElement, Optional[_OnClauseElement], Optional["FromClause"], Dict[str, Any]]
        """
        return any(table.name == table_name for (table, *_) in query._setup_joins)

2. One nit picky thing is that I don't love the join_kwargs name, maybe just joins (since you can have multiple)?

Agreed!

3. I also like that this is backward compatible but we should definitely add something in the docs (yeah I know they could use some love...).
4. Finally, can we add a link to https://docs.sqlalchemy.org/en/20/core/selectable.html#sqlalchemy.sql.expression.Select.join so that people know what parameters are allowed in the dict?

Yes! Where should I add this docs?