Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
26.92k stars 3.84k forks source link

skip and limit is not returning the right document (typescript) #10407

Closed DVGY closed 3 years ago

DVGY commented 3 years ago

Do you want to request a feature or report a bug? BUG

What is the current behavior? skip and limit is returning wrong documents, they are not what is expected

If the current behavior is a bug, please provide the steps to reproduce.

sandbox link

tsconfig

What is the expected behavior? The expected behavior is, the skip should skip the document and show the right result,

How to reproduce the issue

  1. Go to https://xxxxx/api/v1/trips?fields=name,ratingsAverage&paginate=1&limit=10 Above will return 9 docs. This is maximum number of docs I have.

  2. Now use https://xxxxx/api/v1/trips?fields=name,ratingsAverage&paginate=1&limit=2 (1 st page) This will return 2 docs. Observe the name property.

  3. Now use https://xxxxx/api/v1/trips?fields=name,ratingsAverage&paginate=2&limit=2( 2 nd page) This will return 2 docs. Observe the name property.

Try it for page 3 also.

You will notice that the trip name Forest Hiker or The Sea explorer or some name is repeating unnecessary. So I am sometimes getting the same trip name. The trips name should not repeat, until, I start it from page 1 again. Either the skip is not working properly, or there is bug in my code. I tried with raw mongo query, skip and limit works fine.

Pls take a look at tripsController.ts apiFeatures.ts

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version. MongoDB 4,4 Mongoose 5.13.0 Node js 14.16.1

IslandRhythms commented 3 years ago

I think it might be a bug with your code. When limit is set to 1 but paginate is 3, 4, or 5, the name does not change

DVGY commented 3 years ago

I think it might be a bug with your code. When limit is set to 1 but paginate is 3, 4, or 5, the name does not change

I could not find any bug in my code. Yess. @IslandRhythms does that means skip() is not working ?

DVGY commented 3 years ago

I think it might be a bug with your code. When limit is set to 1 but paginate is 3, 4, or 5, the name does not change

export const getAllTrips = catchAsync(
  async (
    req: Request<unknown, unknown, unknown, tripsReqQuery>,
    res: Response,
    next: NextFunction
  ) => {

    const trips = await Trips.find().sort('-createdAt').skip(3).limit(1);

    res.status(200).json({
      status: 'success',
      results: trips.length,
      data: {
        trips,
      },
    });
  }
);

@IslandRhythms I removed my custom logic. Now controller is very simple. It still give me same record everytime. Try tweaking the skip value. I think it is a bug

IslandRhythms commented 3 years ago
const mongoose = require('mongoose');
const {Schema} = mongoose;

const testSchema = new Schema({
    name: String,
    age: Number,
    email: String
}, {timestamps:true});

const Test = mongoose.model('Test', testSchema);

async function test() {

    await mongoose.connect('mongodb://localhost:27017/test', {
        useNewUrlParser: true,
        useUnifiedTopology: true
      });

      await mongoose.connection.dropDatabase();

     for (let i = 0; i < 10; i++) {
        let entry = await Test.create({name: 'Test'+i});
        entry.save();
     }

     console.log(await Test.find().sort('created-at'));
     console.log('=============================================================================');
     console.log(await Test.find().sort('created-at').skip(3).limit(1));
     console.log('=============================================================================');
     console.log(await Test.find().sort('created-at').skip(2).limit(1));
     console.log('=============================================================================');
     console.log(await Test.find().sort('created-at').skip(4).limit(1));
     console.log('=============================================================================');
     console.log(await Test.find().sort('created-at').skip(5).limit(1));
     console.log('=============================================================================');
     console.log(await Test.find().sort('created-at').skip(2).limit(3));
     console.log('=============================================================================');
     console.log(await Test.find().sort('created-at').skip(2).limit(4));
     console.log('=============================================================================');

}

test();
vkarpov15 commented 3 years ago

@IslandRhythms your issue is that you're sorting by created-at, not createdAt.

@DVGY your issue is your tripsModel's createdAt problem:

createdAt: {
      type: Date,
      default: Date.now(),
      select: false
    },

That means every tour has the exact same createdAt, so sort() by createdAt will not give you consistent results. You probably want to do default: () => Date.now() or default: Date.now.

DVGY commented 3 years ago

@vkarpov15 . @IslandRhythms

  1. To be honest it does not even for the property ratingsAverage. I have two tours who ratingsAverage is 4.8 and it fails there.
  2. This works fine in compass. I checked If I use raw mongo db query I get the right results

Update: @vkarpov15 I got from your above comment and here. Thanks

vkarpov15 commented 3 years ago

I took a closer look and I can confirm this is expected behavior. Even with the MongoDB shell I get the ordering issue:

MongoDB Enterprise > db.trips.find({ secretTrip: { $ne: true } }, { name: 1 }).sort({ createdAt: -1 }).skip(2).limit(2)
{ "_id" : ObjectId("5c88fa8cf4afda39709c2955"), "name" : "The Sea Explorer" }
{ "_id" : ObjectId("5c88fa8cf4afda39709c295a"), "name" : "The Snow Adventurer" }
MongoDB Enterprise > db.trips.find({ secretTrip: { $ne: true } }, { name: 1 }).sort({ createdAt: -1 }).skip(0).limit(2)
{ "_id" : ObjectId("5c88fa8cf4afda39709c2955"), "name" : "The Sea Explorer" }
{ "_id" : ObjectId("5c88fa8cf4afda39709c2951"), "name" : "The Forest Hiker" }
MongoDB Enterprise > 

There's no guarantee as to how the MongoDB server breaks ties when sorting. In addition to fixing your schema definition's default createdAt, you should also add _id to your sort() if you have a lot of ties and want to break them:

const trips = await Trips.find().sort({ createdAt: -1, _id: -1 }).skip(3).limit(1);