feathers
feathers copied to clipboard
result.total does not account for params.pipeline
Steps to reproduce
pre.37, db: MongoDB
- Set custom pipeline in a service find before hook like
(context) => { context.params.pipeline = [{ $match: { foo: 'bar' } }] return context }
- Call find() with empty query
The result.data array is correct result.total should be equal to the number of db records where foo is equal to 'bar', but it's equal to the total number of records as if no custom pipeline specified
@xsl777, thanks for reporting. This definitely sounds like a bug, but not the one that you might think. Pagination shouldn't work at all when using the aggregation framework queries.
The aggregation framework can return any type of response, even non-countable and non-page-able ones. So there's not really a reliable way for us to calculate the number of documents. Some queries will return a single document (not in an array) containing a single primitive, and it doesn't even have to match your schema.
So if you're getting a response back that includes pagination, the short-term fix is for us to disable pagination for aggregation framework queries. The long-term fix is to look at the feasibility of performing some query analysis on the pipeline stages to see if we can enable pagination. That will take some research, which is why it's long-term.
This also brings up a good point that I should specify this in the docs.
We might be able to enable pagination whenever the $feathers
stage is used as a first step. We'd also need to look at a way to look ahead in the pipeline operators and disable pagination when other pipeline conditions would break the pagination, though.
The $feathers
stage is handled here: https://github.com/feathersjs/feathers/blob/dove/packages/mongodb/src/adapter.ts#L138-L158
Here's the code causing the bug: https://github.com/feathersjs/feathers/blob/dove/packages/mongodb/src/adapter.ts#L243-L255.
I'm going to see if I can figure out a good way to auto-enable pagination in a reliable way, because I'd love for it to work as you described.
I supposed this line could be updated with the $match
params for simple requests. We could only automatically enable pagination for some of the aggregation stages:
- $match
- $skip
- $sort
- $unset
- $set
- $lookup
- $geoNear
- $fill
Also, there could probably only be one $match
stage.
i have same problem
find(params?: ServiceParams & {
paginate?: PaginationOptions;
}): Promise<Paginated<MediaAd>>;
find(params?: ServiceParams & {
paginate: false;
}): Promise<MediaAd[]>;
async find(params?: any): Promise<Paginated<MediaAd> | MediaAd[]> {
const rsp = await super.find({
...params,
})
if (rsp.total && params && Array.isArray(params.pipeline)) {
const model = await this.getModel(params);
const count = await model.aggregate([
...params.pipeline,
{
$count: "mediaManagerId"
},
{
$project:{
count:'$mediaManagerId'
}
}
])
const rows = await count.toArray()
rsp.total = rows[0]['count']
}
return rsp;
}
I think the solution here is to remove total
from pipeline requests. A separate request for total would need to be run.