mget does not return the correct number of returns if values are undefined or the keys are not in alphabetical order

[eligundry]

Hi, huge fan of your library. I went ahead and forked it to meet the following needs:

• Make it work with cache-manager@5, which simplified the storage contract to be less variadic and more predictable. This doesn't appear to be backwards compatible.
• Converted it to Typescript so that I can get code completion when passing it to cacheManager.caching.
• Swapped in better-sqlite3 for sqlite3 as the former is "faster" and the latter uses a node-gyp fork that has bunch of testing required into code that is shipped with the library which breaks builds in an Astro / Vite setup that I'm working in.

Feel free to port/steal whatever you would like for this library! Open source!

In doing the porting, I noticed a bug in your implementation of mget. The SELECT statement for this is just an IN query which will return unpredictably if keys do not exist OR the keys are not provided in alphabetical order. In practice, the following have issues from what I can tell:

await cache.set('a', 'a')
await cache.set('b', 'b')
await cache.set('z', 'z')

// This mget has a gap at position 'c' and it should return ['a', 'b', undefined, 'z'] but it returns ['a', 'b', 'z']
let [a, b, c, z] = await cache.mget('a', 'b', 'c', 'z')
// this will actually equal 'z'
assert(c === undefined)
// this is undefined as the array is only 3 elements long
assert(z === 'z')

// This mget returns items in reverse order and should be ['z', 'b', 'a'] but it returns ['a', 'b', 'z']
let [z, b, a] = await cache.mget('z', 'b', 'a')
// this will actually equal 'a'
assert(z === 'z')
// this will actually equal 'z'
assert(a === 'a')

The way I got around this is by using a varadic CTE that correctly orders and fills in the gaps in the query with almost equal complexity to an IN statement you have.

> explain query plan WITH getKeys(key) AS (VALUES ("a"), ("b"), ("c"), ("z"))
SELECT
  getKeys.key,
  val,
  created_at,
  expire_at
FROM getKeys
LEFT JOIN kv ON kv.key = getKeys.key;

QUERY PLAN
|--CO-ROUTINE getKeys
|  `--SCAN 3 CONSTANT ROWS
|--SCAN getKeys
`--SEARCH kv USING INDEX sqlite_autoindex_kv_1 (key=?) LEFT-JOIN

> explain query plan select * from kv where key in ("a", "b", "c", "z");

QUERY PLAN
`--SEARCH kv USING INDEX sqlite_autoindex_kv_1 (key=?)

[maxpert]

Great job! I think you should contribute the changes to main library. I am open to PRs. But let's break it down into:

• Typescript conversion
• cache-manager@5 changes
• SELECT query fixes

Would be more than happy to accept PRs and publish newer versions with your contributions.

[eligundry]

I've given this some thought and I'm only going to be interested in contributing upstream if you are okay with the following breaking changes in a major release:

1. Swapping out sqlite3 for better-sqlite3. While neither of their APIs are great and there is a strong argument that sqlite3 has a better API, the fact that it has a gyp dependency that requires a bunch of test libraries is a big deal breaker in the envs that I'm working in (ESM, Vite, Astro)
2. Wrapping msets in a transaction such that if any values are unserializable (or do not pass the provided isCachable callback in the cache-manager@5 contract) the commit is rolled back and an exception is raised. My reasoning on this is that a partial commit and swallowing of the error can lead to some unpredictable behavior that is tricky to catch and that the cache-manager@5 contract does this.
3. This is less important to me, but something I decided to do over in my fork: If a TTL is not provided in a set, mset or default TTL, it will be Infinity. This is similar to how something like Redis would do things (though sqlite does not have the niceties of automatic cache eviction, so maybe this is a terrible idea 😂)

Otherwise, I'm fine fixing the original issue I brought up in this issue and maintaining a fork.

[maxpert]

I think I am fine with changes you are proposing. Would be better for community that they are getting everything from one package rather than forks.