Remove namespace when not passed

[mjesun]

Hi! I'd like to propose a small change related to how cache key computation works: if the namespace is overridden with an empty string, remove it. The goal for that is to allow numerical keys to be used in conjunction with a SQL backed interface, which can significantly speed up things.

[coveralls]

Coverage remained the same at 100.0% when pulling 820144e on mjesun:allow-empty-namespace into 889b0e9 on lukechilds:master.

[lukechilds]

Thanks for this, good stuff.

Do you mind also adding a test to check that by default the keyv namespace is used?

[lukechilds]

On second thoughts, I'm not sure this alone will add any benefit.

The primary key will still be defined as VARCHAR. So even though the value is numerical I think this means it'll be indexed as numeric characters not as an actual number which I think won't give us any speedup.

https://github.com/lukechilds/keyv-sql/blob/ab167866ccb962de4f33a29f2039bfd0b8134caa/src/index.js#L24

We could solve this by adjusting the table schema to use INT primary keys if the namespace is an empty string. However that feels like a bit of a hack, it'd cause breakage, it's not backwards compatible and means keyv.set('foo', 'bar') would cause some nasty DB errors (because 'foo' isn't a valid primary key).

What are your thoughts on this?

I appreciate you taking the time to put this together but personally I think Keyv's focus should be on providing a consistent key-value interface over raw performance. I think for most cases VARCHAR should be fine. Although non breaking perf optimisations are always welcome.

[mjesun]

I'm not planning to change the keyv-sql implementation itself, but to let other implementations be able to get the raw key and do something with it. For instance, one could extend keyv-sql and override the table build, or implement a store from scratch.

Do you mind also adding a test to check that by default the keyv namespace is used?

I can do that if you want to keep the PR :)

[lukechilds]

I'm not planning to change the keyv-sql implementation itself, but to let other implementations be able to get the raw key and do something with it.

Ah, fair enough. Yeah if you could add that extra test I'm happy to merge 👍

[iiAku]

Hello,

Sorry to bring this up. I was wondering what is currently preventing this PR of being merged? I do understand keeping the consistency about how key/value are being names through keyv however this lack of flexibility with the namespace does not help.

[jaredwray]

@mjesun - wanted to check in on the changes that were asked above. Thoughts?

[jaredwray]

going to close this out. Let me know if you still want to work on this and we can reopen it.