Other missing SQLAlchemy features

[igorbenav]

There are a few sql things that are relevant and are currently missing from FastCRUD, I'll list some of them and you guys may add what I forget

• [ ] Add possibility of distinct clause
• [ ] Add group by
• [ ] like, ilike, between operators
• [ ] is_ and isnot_ operators

[JakNowy]

Consider also:

• is_()
• isnot() Very simple, yet potentially useful for None values.

[igorbenav]

Good ones!

[JakNowy]

On the other hand, I love how extendable FastCRUD class is. I was able to easily override create() and get() methods to make EndpointCreator work the way I needed. That's especially true with the introduction of new select() method which is reused across few other methods.

So while flexible FastCRUD is great and highly boosts flexibility of the EndpointCreater, at some point I started wondering if it would be good to incentivize users to override select() method like you often do with get_queryset() in Django. That way we could achieve ultimate flexibility for your queries and get them properly serialized by your Pydantic models, potentially simplifying more complex cases like JoinConfigs for example.

That's probably very conceptual discussion for far future, as the CRUD part is the focus for now, but I'm curious about your thoughts.

[igorbenav]

Totally agree with you, @JakNowy, I think the goal is: if a solution is more complex than just overriding select or even just using sqlalchemy + pydantic, we should reevaluate it. This issue is exactly to do this, understand what's missing and what of the missing stuff should actually be implemented and what we should just document pushing users to create custom solutions.

[JakNowy]

SQLAlchemy2 provides comprehensive list of supported operators. We might pick those which we consider worth supporting. I could volunteer on working on that, I'm just not sure how we would like to tackle cases like distinct() and group_by().

For the endpoint creator, I've issued a PR. It's just a general demonstration, not finished yet, I'm looking forward to hearing your thoughts. I could finalize that after addressing the issue of SQLAlchemy operators and getting aligned on what you'd like to see.

[igorbenav]

SQLAlchemy2 provides comprehensive list of supported operators. We might pick those which we consider worth supporting. I could volunteer on working on that, I'm just not sure how we would like to tackle cases like distinct() and group_by().

Sounds good to me. We may leave the harder stuff aside until it's necessary, maybe by then we'll have a clearer path ahead.

For the endpoint creator, I've issued a PR. It's just a general demonstration, not finished yet, I'm looking forward to hearing your thoughts. I could finalize that after addressing the issue of SQLAlchemy operators and getting aligned on what you'd like to see.

To be honest, I'm not sure I understood exactly where it's headed. Is the Idea for _base_statement to be possibly overwritten, which would apply to other _base_* statements unless these were overwritten as well? (plus the possibility of just overwriting the relevant _base_*_statement without changing _base_statement)

[JakNowy]

Exactly, that's the idea. Let's say in 75% cases generic CRUD will be sufficient. In other 25% cases, user should be able to override the statement (eg. to perform some joins or complex filters). To be honest I thought all the improvements we make in the FastCRUD also apply to EndpointCreator and I was surprised to see that we only support generic get_multi() in _read_items() without filters and joins. I think that's good approach to let the users override that, either with raw SQLAlchemy or other FastCRUD methods.

[JakNowy]

Also the concept of my _base_statement() is not quite right. It's not likely we'll have same statemtents across different methods, unless we wrap them with some select/update statements. We can stick to specific methods.

[igorbenav]

To be honest I thought all the improvements we make in the FastCRUD also apply to EndpointCreator and I was surprised to see that we only support generic get_multi() in _read_items() without filters and joins.

That is the idea eventually, but I'm prioritizing improving FastCRUD methods without necessarily this applying to EndpointCreator right now since (1) it would gatekeep this improvements getting to users for some time and (2) FastCRUD is the base of EndpointCreator, so it makes sense improving it before EndpointCreator.

I think that's good approach to let the users override that, either with raw SQLAlchemy or other FastCRUD methods.

I think it's a good way to overcome while we're not applying this stuff do EndpointCreator.

[igorbenav]

Also the concept of my _base_statement() is not quite right. It's not likely we'll have same statemtents across different methods, unless we wrap them with some select/update statements. We can stick to specific methods.

We could if we wanted to apply a filter or something like it to all endpoints at once.

[JakNowy]

Sounds reasonable to me. I think that's something for you to think about and make a final design decision.

When it comes to this issue, I'm going to implement it this week, also including startswith, endswith, contains. For now, I'll follow the convention and simply add mappings for them. We might try going for something more fancy in the future if you'd like.

[igorbenav]

Sure. I think the base_statement to apply something to all endpoints is useful, but if we're encouraging users to tweak these things, maybe naming it as private stuff isn't a good approach. Thinking that a user should be able to change this stuff.

class EndpointCreator:
    def __init__(self, model, db_session: AsyncSession, query_configs=None):
        self.model = model
        self.db_session = db_session
        self.query_configs = query_configs or {}

    def default_read_item_query(self):
        return select(self.model)

    def default_read_item_query(self):
        return self.default_base_query()

    def default_create_item_query(self):
        return self.default_base_query()

    def default_update_item_query(self):
        return self.default_base_query()

    def default_delete_item_query(self):
        return self.default_base_query()

    def get_base_query(self, operation_type):
        if operation_type in self.query_configs:
            return self.query_configs[operation_type]
        else:
            return getattr(self, f"default_{operation_type}_query")()

    def _read_item(self):
        """Creates an endpoint for reading a single item from the database."""
        @apply_model_pk(**self._primary_keys_types)
        async def endpoint(db: AsyncSession = Depends(self.session), **pkeys):
            base_query = self.get_base_query('read_item')
            for key, value in pkeys.items():
                base_query = base_query.where(getattr(self.model, key) == value)
            
            result = await db.execute(base_query)
            item = result.scalars().first()
            if not item:  # pragma: no cover
                raise NotFoundException(detail="Item not found")
            return item  # pragma: no cover

        return endpoint

Maybe something like this. Btw, this would close #41. I think I'll start looking at #15