key behaviour after msgpackr error 95

[1N50MN14]

One small issue, I totally missed Date wasn't within the msgpack serialization spec, and passed some old test that contained a value like so {dummy:new Date()}. That triggered the error Error: In database data: Unknown extension type 95 as intended.

Problem is, any call to that specific key thereafter, be put or get throws the same error the only way I was able to write to that specific key again was to remove it.

[1N50MN14]

Another minor issue:

db.remove(key, ifVersion) followed by getLastVersion() does not reset the version, instead it returns the last version of the deleted key (even with a delay between the two operations), which can be problematic for future "inserts" aka db.put(key, value, 1, null). However, if I remove the key again, then ifValue resets to zero.

Speaking of which db.put(key, value, 1, 0) is always falsy, took me a minute before I figured it should be null, I added an ifValue check on all operations. Just a small usability feedback.

[kriszyp]

I pushed an update for a fix for the date issue (version 0.6.1). I believe the dates should be serialized properly (using the standard MessagePack extension), and the fix should be able to read any dates that you have written to the db. Note, that it currently uses the 32-bit version of dates, which drops milliseconds, hopefully that's ok.

As far as getLastVersion behavior, I had just intended it would return the last version for get() calls. Were you expecting it to also return the version of the last put as well? I could certainly do that, it is just beyond the original intent, but that is more intuitive, that's fine.

When you do db.put(key, value, 1, 0), were you intending that to succeed when there was no existing entry (rather than only succeeding if there was an existing entry with a version of 0)?

[1N50MN14]

The intended behavior of getLastVersion() is absolutely correct, and the one that makes sense the most. The issue is that it doesn't behave as intended when preceded by a get on a non existent key that has just been removed using remove. To illustrate this:

• db.get(nonExistentKey) followed by getLastVersion() returns a version value of 0 - correct
• db.get(existentKey) followed by getLastVersion() returns whichever version value the key has, say for example 6 - correct

But the following:

• db.remove(existentKey), followed by db.get(existentKeyWeJustRemoved) followed by getLastVersion() returns version value of 6 instead of 0.

There interesting thing is that adding db.insert(existentKeyWeJustRemoved, value, 1, null) (or 0 in 0.6.1) to the above sequence does not fail. I'm using the version in some cases to react to record changes (it's not a biggie, I'm checking the actual value as well to be on the safe side).

When it comes to the dates I'm manually handling that (need ms in some cases), still, the 32-bit version of dates is a super welcome addition in case I / someone else makes the same mistake again there's a some form of protection in place.

Many thanks for the fixes and the follow up!

[kriszyp]

Ah, thank you for the explanation! I pushed a fix in 0.6.2 that should return null from getLastVersion() after a get(nonExistentKey) (to make it symmetrical with what you would provide for an ifVersion if you expect the key not to exist).

I also added support for full timestamps in msgpackr so milliseconds can be recorded.

[1N50MN14]

@kriszyp Oh that was quick! I've just upgraded to 0.6.2 re-ran tests and they all passed like a charm! love love all of the recent additions to lmdb-store!

Hitting that "Close with comment" button, many thanks once again and stay safe!