Aggregations should be able to handle `Sort.unsorted()` about spring-data-mongodb HOT 5 OPEN

Thank you @bithazard for getting in touch. We'll look into this.

from spring-data-mongodb.

I've been looking at the mongodb documentation and related tickets but there's no way to pass in something similar to $natural sorting when using aggregations. We cannot silently ignore the stage if unsorted is given to it nor would I want to put in a hard guard that raises an error.
Updating the documentation might help raising awareness.

from spring-data-mongodb.

I thought into this direction as well. Sorting by id would probably come as close as possible to a "$natural sorting". But that could lead to some unwanted side effects elsewhere (not to mention that it would fail for documents without id). So this is clearly not an option.

Of course the issue could be solved on the MongoDB side. They could simply allow sort: {} / sort: null as valid syntax for an aggregation. It is valid for a find as well (e.g. {"find": "test", "sort": {}} or even db.test.find({}, {}, { "sort": null })). But then again that would not really help us here, as it would only work with MongoDB versions after the change.

Regarding the "silently ignore the stage": I think it would be more like skipping the stage because there is simply nothing to do. I understand that it's not great to add a special case for such a situation but I also looked a bit into the code and I think it would be possible to add a check to https://github.com/spring-projects/spring-data-mongodb/blob/main/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java#L56 to handle this.

There is a loop that iterates over all AggregationOperations. There are even already some instanceof checks. If we check if operation is an instance of SortOperation we could conditionally add or not add it to operationDocuments. But for this we would need some way to figure out whether the SortOperation is unsorted. Currently SortOperation is very private. We could either add an isUnsorted() method which simply passes the call through to sort.isUnsorted() or we could override equals so that we can check sortOperation.equals(new SortOperation(Sort.unsorted())).

Just a thought. I don't know the code well enough to say if that would cause issues elsewhere.

from spring-data-mongodb.

Thanks for the feedback - the checks in AggregationOperationRenderer relate to the context the next stage might operate on, not so much of handling specifics of a stage and we do not want to scatter logic throughout different code paths.
While we can decide to skip stages for annotated aggregations on repository level, those explicitly provided by the user and fed to the template should be treated as given with all the implications (like the server error) this may have.
Maybe it's worth asking the MongoDB team to consider adding something like a $natural to aggregations.

from spring-data-mongodb.

I've added a suggestion to the "MongoDB Feedback Engine" that you can find under https://feedback.mongodb.com/forums/924280-database/suggestions/48473684-aggregations-should-allow-an-empty-sort-stage-inst. Feel free to give it a vote.

If this gets implemented, the way that you currently generate the sort stage, will start working. I think that it also makes sense for MongoDB to handle an empty sort stage like this. A sort object with n keys will sort by these n fields, so an empty object should not sort by any field. Adding another special keyword (like $natural) would just add more (unnecessary) complexity. And you still would have to check if the sort object is empty and only in this case add a {"$natural": 1} (or something like that) to the object.

I also found this section of the MongoDB sort aggregation documentation, where they recommend to add the _id field to the sort object if you want to achieve sort consistency. I think sort consistency is always a good thing and if you get it almost for free (as the _id is always indexed), I don't see any reason not to add it. So as a workaround one could always add the _id as sort field like this:

    //Some sort object you get from somewhere (could be Sort.unsorted())
    Sort sort = ...;
    
    mongoTemplate.aggregate(newAggregation(sort(sort.and(Sort.by("_id")))), "test", Document.class);

from spring-data-mongodb.