bwgjoseph / mongoose-vs-ottoman

feature comparison between mongoose and ottoman
0 stars 1 forks source link

lean (regression) #114

Open bwgjoseph opened 2 years ago

bwgjoseph commented 2 years ago

Hi,

Congrats on going GA on ottoman!

While testing with the latest driver, seem like something ain't right with lean method. The document size for lean is larger than non-lean.

const leanDoc = await Airplane.findById(created.id, { lean: true });
const nonLeanDoc = await Airplane.findById(created.id);
const sizeOfLeanDoc = sizeof(leanDoc);
const sizeOfNonLeanDoc = sizeof(nonLeanDoc);
console.log(sizeOfLeanDoc); // 670
console.log(sizeOfNonLeanDoc); // 574
assert.ok(sizeOfLeanDoc <= sizeOfNonLeanDoc);

Has been working fine up to beta.9

Thanks

AV25242 commented 2 years ago

Thanks @bwgjoseph , wouldn't be possible with your support that goes without saying. You have been a great partner and collaborator.

We will take a look into this

ariamF11 commented 2 years ago

Hi @bwgjoseph , what happens is a problem of the sizeof library with the Date type properties:

Let's see a simple example code before:

let a = { date: new Date('20 Nov 2020 11:30') };
console.log(a.date instanceof Date); // true
console.log(typeof a); // object
let b = JSON.parse(JSON.stringify(a)); // simulate `lean` over `a`
console.log(a.date instanceof Date); // false cause b is now a POJO
console.log(typeof b.date); // string

Now destructuring sizeof operations with next object structure, extracted from your example: { scheduledAt, id, _type }

will get:

*- This is the main object properties names: stringProperties(MainObject): [ "scheduledAt", "id", "_type" ]

// WHITOUT LEAN

CalcPropertyName(scheduledAt): 22

// This is where scheduleAt breaks the sizeof algorithm cause scheduledAt is an instance of Date and apply Object recursion Object(scheduledAt): "2020-11-20T16:30:00.000Z"

*- Recursive call to scheduledAt as Date Object PropertiesOf(scheduledAt): [] // cause Date not have properties using forin loop like sizeof do CalcPropertyValue(scheduledAt): 0

String(id): "id" CalcPropertyName(id): 4

String(id): "d4358a28-54f5-40e9-9e5b-d14346166f1e" CalcPropertyValue(id): 72

String(_type): "_type" CalcPropertyName(_type): 10

String(_type): "Airplane" CalcPropertyValue(_type): 16

TOTAL: 124

// WITH LEAN

String(scheduledAt): "scheduledAt" CalcPropertyName(scheduledAt): 22

// This is where sizeof algorithm works fine cause scheduledAt is a String String(scheduledAt): "2020-11-20T16:30:00.000Z" CalcPropertyValue(scheduledAt): 48

String(id): "id" CalcPropertyName(id): 4

String(id): "f2cd1edd-6316-420a-8f9e-ce47bd17c329" CalcPropertyValue(id): 72

String(_type): "_type" CalcPropertyName(_type): 10

String(_type): "Airplane" CalcPropertyValue(_type): 16

TOTAL: 172

In your example you have two properties that are instances of Date, they are scheduledAt and info.firstFlightAt then we get *248 = 96 then if you do this: sizeOfLeanDoc - sizeOfNonLeanDoc = 96 ** and there is the diference between lean and not lean in your example

bwgjoseph commented 2 years ago

Hey @gsi-ariam,

Thank you for taking your time to explain this. Appreciate it.

But what cause it to break given that the code don't change except to upgrade ottoman version? Wouldn't that mean this is somehow caused by some changes in ottoman? Or has the previous version been wrong all along until the recent changes correct the behavior and now it's reporting as it should have?

ariamF11 commented 2 years ago

:wave: @bwgjoseph yes to your second question

the previous version has been wrong all along until the recent changes correct the behavior and now it's reporting as it should have

:+1::+1::+1:

bwgjoseph commented 2 years ago

Thanks, in that case, what's the best way to assert this if that library has issue?

I've tried various ways (researching online), and each solution kind of gives me different answer (in the size it returns)