jaredwray / keyv

Simple key-value storage with support for multiple backends
https://keyv.org
MIT License
2.61k stars 143 forks source link

Namespace mix-up when using iterators and SQLite #1160

Closed dtinth closed 2 weeks ago

dtinth commented 2 weeks ago

Describe the bug When using the same store with multiple namespaces, all instances of the stores will use the namespace of the latest created instance.

How To Reproduce (best to provide workable code or tests!)

import KeyvSqlite from "@keyv/sqlite";
import Keyv from "keyv";

const store = new KeyvSqlite("sqlite://.data/test.db");

const keyvA = new Keyv({ store, namespace: "a" });
const keyvB = new Keyv({ store, namespace: "b" });

async function main() {
  await keyvA.set("a", "x");
  await keyvA.set("b", "y");
  await keyvA.set("c", "z");

  await keyvB.set("a", "one");
  await keyvB.set("b", "two");
  await keyvB.set("c", "three");

  console.log("=== From A ===");
  for await (const [key, value] of keyvA.iterator()) {
    console.log(key, value);
  }
  console.log("=== From B ===");
  for await (const [key, value] of keyvB.iterator()) {
    console.log(key, value);
  }
}

main();

Expected output:

=== From A ===
a x
b y
c z
=== From B ===
a one
b two
c three

Actual output:

=== From A ===
a one
b two
c three
=== From B ===
a one
b two
c three
jaredwray commented 2 weeks ago

@dtinth - thanks so much for letting us know on this. I am going to get a unit test together on this to figure out what it is doing.

jaredwray commented 2 weeks ago

@dtinth - looks like the issue is that you are running a single Storage Adapter which means that when Keyv sets the namespace on iterator to do the query it causes an issue because it is not updating the namespace on generation. To work around this the fix is pretty simple:

import KeyvSqlite from "@keyv/sqlite";
import Keyv from "keyv";
const keyvA = new Keyv({ store: new KeyvSqlite("sqlite://.data/test.db"), namespace: "a" });
const keyvB = new Keyv({ store: new KeyvSqlite("sqlite://.data/test.db"), namespace: "b" });

async function main() {
  await keyvA.set("a", "x");
  await keyvA.set("b", "y");
  await keyvA.set("c", "z");

  await keyvB.set("a", "one");
  await keyvB.set("b", "two");
  await keyvB.set("c", "three");

  console.log("=== From A ===");
  for await (const [key, value] of keyvA.iterator()) {
    console.log(key, value);
  }
  console.log("=== From B ===");
  for await (const [key, value] of keyvB.iterator()) {
    console.log(key, value);
  }
}

main();

I am going to be updating how namespace works in the next month or two and will make it easier to handle this scenario in the future.

dtinth commented 2 weeks ago

@jaredwray Hmm, doesn’t this create 2 separate connections to the underlying database in this case? My use case involves creating namespaces dynamically, so there can be over thousand namespaces, I don’t think it’s a good idea to create a new SQLite3 instance for every namespace.

I’ve already worked around this issue by asking Claude to reimplement Keyv’s API surface area with raw SQLite, but it’s going to be inconvenient should I want to switch to a different database. I’ll look forward to improvements on how namespace works so that I can migrate back to Keyv, thanks!

jaredwray commented 2 weeks ago

That is correct and if you are doing thousands of namespaces you might want to look for something different.