Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLite API #11657

Open
tracker1 opened this issue Aug 11, 2021 · 28 comments
Open

SQLite API #11657

tracker1 opened this issue Aug 11, 2021 · 28 comments
Labels
needs discussion this topic needs further discussion to determine what action to take suggestion suggestions for new features (yet to be agreed)

Comments

@tracker1
Copy link

Since SQLite is being used to back WebStorage, it would be nice if this were exposed. Since the engine is already embedded, and the process for including binary modules is pretty cumbersome, it would be nice to have an access for SQLite closer to directly. Would also be able to keep within the read-write permission constraints this way as well, where a binary module could/would be more relaxed.

While deprecated, the WebSQL API might be a good option for this. I have made this suggestion previously, there still isn't a good story, especially if you need multi-process access to a database on disk, so the wasm module is inappropriate for many scenarios.

All query/exec methods are template string processors.

// open a database file/connection, create if it doesn't exist
const db: SqliteDatabase = Deno.openDatabase('~/foo.db');

// execute without a result
await db.exec`CREATE TABLE ...`;

// query to an array of results.
const fooResults: any[] = await db.query`SELECT * FROM foo WHERE bar = ${baz}`;

// query to an iterable
for (const record: any of db.iterate`...`) {
  // ...
}

// query to an async iterable
for await (const record: any of db.iterateAsync`...`) {
}

// multi-step transaction...
await db.useTransaction(async (t: SqliteTransaction /* same as DB + .commit and .rollback methods */) => {
  // rolls back automatically when rejected and not already committed
  try {
    ...
    t.commit();
  } catch(error) {
    t.rollback();
  }
});
@bartlomieju
Copy link
Member

This has been discussed extensively by the core team in recent weeks. We won't expose bindings directly on the Deno namespace, but want to move https://deno.land/x/sqlite@v3.0.0 into deno_std (@dyedgreen already expressed interest in this scenario). Right now it is blocked on #11511 which will add required APIs to ensure sqlite can be used to its fullest.

In addition to that IndexedDB should make its way into Deno in the coming months as well.

@ghost
Copy link

ghost commented Jan 15, 2022

Will the move into deno_std let me use native SQLite bindings instead of the WASM compiled version?

@andykais
Copy link

andykais commented May 5, 2022

@waynebloss it will not, what @bartlomieju commented on is moving https://deno.land/x/sqlite@v3.0.0 under the std namespace.

For what its worth, if there was a vote on whether we should expose bindings or move a wasm implementation into std, I would choose the former. I am not a contributor though, so I respect that this is a decision of the owners of deno 🙂. If sqlite were a standard web interface, this would be a much simpler conversation

@dyedgreen
Copy link
Contributor

I think its tricky. What’s really nice about the WASM based solution is that it has a clear runtime-independent API it expects. This API can then be provided in e.g. deno deploy, but its also possible to use the same library in a web browser etc.

On the other hand, piping all the IO through a JS translation layer is pretty expensive.

@ghost
Copy link

ghost commented May 5, 2022

Thanks. Yeah I was just wondering - I suppose it won't much matter where I'm importing it from.

I'm glad the native binding is there when it's necessary and it seems like sqlite3 has some good activity happening.

@dyedgreen
Copy link
Contributor

Yes sqlite3 has a nice approach. But I’d suggest to use the WASM bindings unless you run into performance problems in your use-case.

They have the major advantages of a) preserving the deno security model and b) preserving consistency across deploys / environments (ie they provide a specific version of SQLite not just whichever happens to be installed by your system)

@andykais
Copy link

andykais commented May 9, 2022

Alright, so whether or not the following information is useful, I am dumping it here anyways because I have done a bit of reading on past, present and future web db stories.

The past

It looks like there was an attempt to expose sqlite in the browser back in 2010, but the specification flopped because sqlite was a single implementation, not a standard interface https://www.w3.org/TR/webdatabase/

This document was on the W3C Recommendation track but specification work has stopped. The specification reached an impasse: all interested implementors have used the same SQL backend (Sqlite), but we need multiple independent implementations to proceed along a standardisation path.

The present

So node.js has sql.js (another wasm sqlite lib) and absurd-sql, which use IndexedDB as a backend.

@dyedgreen's deno-sqlite appears to also expose a browser interface that has persistent writes, using IndexedDB as a backend. Very cool, you appear to have feature parity with the other existing wasm sqlite libraries.

Personally, I do not have a need for browser-compatible database drivers. If I am using sqlite in a deno/nodejs project, it is because I want the data to be portable to other tools, not the implementation. It is important therefore that my data be persisted to the filesystem, not stored in IndexedDB.

benchmarks

The ffi lib wins out in speed in most cases, except select many queries that return objects (likely because there isnt a native solution yet, it just uses .step() and creates the js objects one at a time).

benchmark results
cpu: Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
runtime: deno 1.21.2 (x86_64-apple-darwin)

file:///Users/andrew/Code/scratchwork/deno-benchmark-sqlite/bench.ts
benchmark                                     time (avg)             (min … max)       p75       p99      p995
-------------------------------------------------------------------------------- -----------------------------
deno.land/x/sqlite (wasm) - select count  221.14 µs/iter   (169.68 µs … 2.07 ms) 236.75 µs 342.55 µs 399.53 µs
deno.land/x/sqlite3 (ffi) - select count   30.84 µs/iter    (24.85 µs … 1.56 ms)  31.98 µs  77.11 µs 110.62 µs

summary
  deno.land/x/sqlite3 (ffi) - select count
   7.17x times faster than deno.land/x/sqlite (wasm) - select count

deno.land/x/sqlite (wasm) - select many   305.56 µs/iter   (228.18 µs … 1.64 ms)  326.8 µs 732.41 µs 790.12 µs
deno.land/x/sqlite3 (ffi) - select many     2.58 ms/iter      (2.3 ms … 3.56 ms)   2.74 ms   3.17 ms   3.37 ms

summary
  deno.land/x/sqlite (wasm) - select many
   8.44x times faster than deno.land/x/sqlite3 (ffi) - select many

deno.land/x/sqlite (wasm) - select one    217.95 µs/iter   (167.94 µs … 2.88 ms) 229.15 µs 351.45 µs 418.38 µs
deno.land/x/sqlite3 (ffi) - select one     54.88 µs/iter    (40.22 µs … 2.03 ms)  53.96 µs  100.4 µs 194.46 µs

summary
  deno.land/x/sqlite3 (ffi) - select one
   3.97x times faster than deno.land/x/sqlite (wasm) - select one

deno.land/x/sqlite (wasm) - insert one     45.97 ms/iter    (42.9 ms … 62.88 ms)  44.36 ms  62.88 ms  62.88 ms
deno.land/x/sqlite3 (ffi) - insert one    105.98 µs/iter   (67.36 µs … 30.57 ms)   87.5 µs 208.53 µs 553.65 µs

summary
  deno.land/x/sqlite3 (ffi) - insert one
   433.78x times faster than deno.land/x/sqlite (wasm) - insert one
benchmark code
import * as fs from "https://deno.land/std@0.138.0/fs/mod.ts";
import * as SQLITE_WASM from "https://deno.land/x/sqlite/mod.ts";
import * as SQLITE_FFI from "https://deno.land/x/sqlite3@0.4.2/mod.ts";

const SELECT_MANY_SEED_COUNT = 10000


const deno_bench = Deno.bench


export interface Statement {
  execute(...params: unknown[]): void;
  query_one(...params: unknown[]): any
  query_many(...params: unknown[]): unknown[]
  finalize(): void;
}

type Params = {[key: string]: any}
abstract class Backend<DB> {
  public abstract backend_name: string
  protected db: DB
  constructor(protected db_path: string) {
    if (fs.existsSync(db_path)) Deno.removeSync(db_path)
    if (fs.existsSync(db_path+"-wal")) Deno.removeSync(db_path+"-wal")
    if (fs.existsSync(db_path+"-shm")) Deno.removeSync(db_path+"-shm")
    this.db = this.create_db(db_path)
  }
  abstract create_db(db_path: string): DB
  abstract prepare(sql: string): Statement
  abstract execute(sql: string): void
  abstract query(sql: string): unknown[]
  abstract close(): void
}


class SqliteWASM extends Backend<SQLITE_WASM.DB> {
  backend_name = "deno.land/x/sqlite (wasm)"

  create_db(db_path: string) { return new SQLITE_WASM.DB(db_path) }

  prepare(sql: string) {
    const stmt = this.db.prepareQuery(sql);
    return {
      execute: (params: Params) => { stmt.execute(params as any) },
      query_one: (params: Params) => stmt.allEntries(params)[0],
      query_many: (params: Params) => stmt.allEntries(params),
      finalize: () => stmt.finalize(),
    };
  }

  execute(sql: string) {
    this.db.query(sql)
  }

  query(sql: string) {
    return this.db.query(sql)
  }

  close() { this.db.close() }
}


class SqliteFFI extends Backend<SQLITE_FFI.Database> {
  backend_name = "deno.land/x/sqlite3 (ffi)"

  create_db(db_path: string) { return new SQLITE_FFI.Database(db_path) }

  prepare(sql: string) {
    const stmt = this.db.prepare(sql)
    return {
      execute: (params: Params) => {
        stmt.execute(params)
      },
      query_one: (params: Params) => {
        stmt.bindAllNamed(params)
        stmt.step()
        const row = stmt.row.asObject()
        stmt.reset()
        return row
      },
      query_many: (params: Params) => {
        return this.db.queryObject(sql, params)
        // const rows = []
        // stmt.bindAllNamed(params)
        // let row
        // while (row = stmt.step()) {
        //   rows.push(row.asObject())
        // }
        // stmt.reset()
        // return rows
      },
      finalize:() => stmt.finalize()
    }
  }

  execute(sql: string) {
    this.db.execute(sql)
  }

  query(sql: string) {
    return this.db.queryArray(sql)
  }

  close() { this.db.close() }
}

function benchmark(db: Backend<unknown>) {
  db.execute(`
    CREATE TABLE basic (
      id INTEGER PRIMARY KEY NOT NULL,
      float REAL NOT NULL,
      big_chunk BLOB NOT NULL
    );
  `)
  db.execute(`CREATE INDEX float_index ON basic(float);`)
  db.execute('PRAGMA journal_mode = WAL')
  db.execute('PRAGMA synchronous = normal')
  db.execute('PRAGMA temp_store = memory')
  db.execute('PRAGMA page_size = 512')
  db.execute('PRAGMA cache_size = 4000')

  const insert_stmt = db.prepare(`INSERT INTO basic (float, big_chunk) VALUES (:float, :big_chunk)`)
  const select_many_stmt = db.prepare(`SELECT * FROM basic`)
  const select_one_stmt = db.prepare(`SELECT * FROM basic WHERE float <= :float LIMIT 1`)
  const count_stmt = db.prepare(`SELECT COUNT(*) AS count FROM basic`)

  db.execute(`DELETE FROM basic`)
  for (let i = 0; i < SELECT_MANY_SEED_COUNT; i++) insert_stmt.execute({ float: Math.random(), big_chunk: '' })

  deno_bench({
    name: `${db.backend_name} - select count`,
    group: "select count",
    fn: () => {
      const {count} = count_stmt.query_one()
      if (count !== SELECT_MANY_SEED_COUNT) throw new Error(`unexpected count ${count}`)
    }
  })

  deno_bench({
    name: `${db.backend_name} - select many`,
    group: "select many",
    fn: () => {
      const rows = select_many_stmt.query_many()
    }
  })

  deno_bench({
    name: `${db.backend_name} - select one`,
    group: "select one",
    fn: () => {
      const row = select_one_stmt.query_one({ float: Math.random() })
    }
  })

  deno_bench({
    name: `${db.backend_name} - insert one`,
    group: "insert one",
    fn: () => {
      insert_stmt.execute({float: Math.random(), big_chunk: ''})
    }
  })
}

const sqlite_wasm = new SqliteWASM('sqlite_wasm.db')
const sqlite_ffi = new SqliteFFI('sqlite_ffi.db')

benchmark(sqlite_wasm)
benchmark(sqlite_ffi)

I dont have a great way to benchmark better-sqlite3 (a npm library) inside deno bench, but I suspect it is slightly faster than FFI, since the bindings are more closely tied to the implementation. (the async-sqlite3 deno library does indeed show this benchmark, though I dont know how exactly it was tested).

Part of this may have to do with WebAssembly being limited to 4GB of memory, though I suspect that WASM just runs slower than native in a lot of cases. There is a writeup on why the 4GB limitation exists (currently, but not forever!) here https://v8.dev/blog/4gb-wasm-memory.

The future

It looks like some database-friendly file system apis are coming to the browser soon https://web.dev/file-system-access/#accessing-files-optimized-for-performance-from-the-origin-private-file-system, which could give developers the best of both worlds (e.g. portability of implementation, and of data)

This is where the Storage Foundation API comes in. The Storage Foundation API is a new fast and unopinionated storage API that unlocks new and much-requested use cases for the web, such as implementing performant databases and gracefully managing large temporary files. With this new interface, developers can "bring their own storage" to the web, reducing the feature gap between web and platform-specific code.

WebAssembly also has a proposal for essentially unbounded RAM usage in Stage 3 https://github.com/WebAssembly/proposals#phase-3---implementation-phase-cg--wg (it is called memory64)

My Two Cents

Both portability and performance are important pillars for building resilient, easily distributed js apps. Currently, a wasm approach gives us portability at a cost to performance. The FFI approach gives us the opposite, performance at the cost of portability. A native-builtin deno solution would give us the best of both worlds, but it comes with its own pain points for the core deno team. E.g. (maintenance, polluting the codebase, lack of flexibility in sqlite extensions & versions).

@dyedgreen
Copy link
Contributor

Personally, I do not have a need for browser-compatible database drivers. If I am using sqlite in a deno/nodejs project, it is because I want the data to be portable to other tools, not the implementation. It is important therefore that my data be persisted to the filesystem, not stored in IndexedDB.

To add a note to this: The index db is used purely as a persistent layer, basically as a key-value store from 'file name' to an opaque Uint8Array, which holds the sqlite database.

This representation is fully compatible, i.e. all you have to do is load the data out of the indexed db and into a file to read it elsewhere. (E.g. https://dyedgreen.github.io/tasks/dist/ has a backup button, which just downloads the database file for further use)

@dyedgreen
Copy link
Contributor

Maybe another issue to mention with having a built-in SQLite / using a native SQLite with potentially untrusted code, is that it would break Deno's sandbox: https://sqlite.org/appfunc.html#security_implications

@andykais
Copy link

dang that isnt something I considered...though I guess it depends on what kind of api is exposed. I imagine if registered functions were restricted to js function, then deno would still own the runtime?

@dyedgreen
Copy link
Contributor

Yeah, but generally, having SQLite (i.e. a more or less opaque blob of C code) interact with the file-system and os means that there is a lot more surface for permissions etc to go wrong. E.g. how do you prevent people from making a database file have write access. How do you make sure only files allow-listed directories are read, etc ...

(also see https://sqlite.org/security.html)

@andykais
Copy link

andykais commented Jun 2, 2022

That is a fair point that I hadnt considered. I imagine code execution is a compiler flag though. I think outside of that deno could handle read/write permissions on the few well defined sqlite files that it needs to access (e.g. sqlite.db, sqlite.db-wal, sqlite.db-shm)

@littledivy
Copy link
Member

I and @bartlomieju are supportive of native SQLite bindings. We've already discussed this internally multiple times and ended up not doing it but there is enough community feedback to assume that this is wanted.

I'm going to open it again for discussion.

@littledivy littledivy reopened this Aug 1, 2022
@littledivy
Copy link
Member

Regarding performance, I'm currently working on leveraging fast FFI calls in @DjDeveloperr's x/sqlite3 (Eg: denodrivers/sqlite3#40).

Initial results show that querying data are almost 5x faster than Node's better-sqlite3.

The x/sqlite Wasm module can also be made fast by optimizing the Wasm<->JS binding layer, maybe implementing Wasi natively so it could leverage auto Wasm entry optimization for V8 fast API calls.

@littledivy littledivy added suggestion suggestions for new features (yet to be agreed) needs discussion this topic needs further discussion to determine what action to take labels Aug 1, 2022
@kitsonk
Copy link
Contributor

kitsonk commented Aug 1, 2022

And I am not supportive. We have for a long time provided a runtime where any non standard APIs are kept in the Deno namespaces. We have focused being on a standards compliant runtime. We have even walked back exposing APIs that are non-standard to keep it manageable, like Deno.emit().

Adding non-standard APIs only fractures the ecosystem. It causes people to write code that only works on a single runtime. We shouldn't further fracture the ecosystem.

I can understand a desire to do cool things, but history over the last 20 years of the JavaScript ecosystem demonstrates that these ideas never end up going well. We should do things that are part of a standard based approach.

Also, while we shouldn't make a decision purely based on Deno Deploy, we need to honestly consider if this is something we introduce that fractures the workloads you can run on Deploy versus CLI that are built-in.

Keeping it as an official maintained WASM module, or a FFI plugin is in my opinion the best approach. It decoupled the runtime from a specific version of SQLite as well as makes it totally opt-in.

There is a web standard, IndexedDB, that we have failed to deliver. Delivering IndexedDB natively in CLI and Deploy would allow a lot of existing code to just work. We really should be pulling out all stops to deliver it. Yes, SQLite is more popular, yes WebSQL was a better idea, but that argument was had a decade a go and the web standard is IndexedDB, something we should absolutely have in Deno.

@sant123
Copy link

sant123 commented Aug 2, 2022

Keeping it as an official maintained WASM module, or a FFI plugin is in my opinion the best approach. It decoupled the runtime from a specific version of SQLite as well as makes it totally opt-in.

This sounds interesting, since a standard library module can be updated standalone instead of a next Deno release 🤔 however not pretty sure which alternative could have more performance; if WASM of FFI? Also FFI currently is unstable so it will require using the flag while using the module.

@andykais
Copy link

andykais commented Aug 9, 2022

Since the discussion is opened up again, I will make my case again.

addressing design pains

I can understand a desire to do cool things, but history over the last 20 years of the JavaScript ecosystem demonstrates that these ideas never end up going well. We should do things that are part of a standard based approach. - @kitsonk

I understand the argument against non-standard apis. It is fair to say that adding sqlite natively will cause a decent amount of bikeshedding regarding the "right" sqlite version to expose from deno. I dont think that should be a blocker though. I think reaching a consensus on a sqlite version that covers 90% of the use cases of sqlite should be pretty easy. Deno also has an unstable flag, so it should be easy enough to expose sqlite for a few releases, gather feedback, and then harden the api.

Deno.run isnt a web standard, and neither is Deno.dlopen, they are both just two very useful apis.

One more followup here, the last 10% of the developer community will always still have the option to roll their own sqlite ffi lib or wasm lib if they want some very specific sqlite features that deno doesnt provide standard.

addressing fracturing the runtime

Adding non-standard APIs only fractures the ecosystem. It causes people to write code that only works on a single runtime. We shouldn't further fracture the ecosystem. - @kitsonk

so, I understand this argument, but at the same time, the runtime is already fractured, and its a good thing. I can do things like Deno.run or Deno.dlopen within the cli context, but not from deno deploy. Developers also write code in deno that is used within nodejs or the browser. Sometimes this is as simple as typeof Deno !== undefined, other times we use dnt to change the code to be compatible with a different environment. I suppose some of these are the 'deno' runtime, but we develop for them within the deno runtime.

addressing security

Yeah, but generally, having SQLite (i.e. a more or less opaque blob of C code) interact with the file-system and os means that there is a lot more surface for permissions etc to go wrong. E.g. how do you prevent people from making a database file have write access. How do you make sure only files allow-listed directories are read, etc ... - @dyedgreen

I dont like the idea of breaking deno's sandbox, but both --allow-ffi and --allow-run already break it. Originally I had imagined that deno could make some assumptions about what files sqlite will open, but there is always the option to add --allow-sqlite if deno truly doesnt trust this library. That said, sqlite is a very well trusted piece of software. It comes standard in many linux distributions and disabling some of the more dangerous features might be enough to make some assumptions. Also, does deno not have any c/c++ libraries compiled into it? I suppose the issue is exposing a c lib, because I would almost guarantee that theres a c library required by one of the rust libs used by deno somewhere.

advocating based on prior art

https://bun.sh Personally, I think that this project just wont have the same impact that deno has on the javascript community. It seems like there are less hard rules about its design, and instead it just picks and chooses what works well. That said, there could be something to be said that sqlite is a native addition to the runtime. Another runtime that includes sqlite natively is python! I will admit that python has a pretty weird sqlite api, but you dont see the community complaining too often that they wish it wasnt there.

I feel like this is a strong argument against some of the web standards argument. A big strength of deno is its "batteries included" nature. The standard lib is invaluable, and certainly makes some assumptions about what good library apis look like. The difference here is just the location of this standard tool, being inside the runtime itself rather than in a standard lib

advocating for performance

Ill be honest, I think that we have people working hard at both ffi and wasm implementations of sqlite, but I just dont see a way that those will reach the same speeds that a maintained internal sqlite will reach. I want to use deno for extremely low latency tools for websocket/udp/tcp applications. The better performance on the database layer, the better the application. You would be amazed how much traffic you can get away with on a single web server with a good sqlite driver.

ok, do we need a standard?

This is really the most obnoxious thought here, but if it is going to take a web standard to get this feature into deno, could deno back the idea of a sqlite web standard? I believe I remember a blog post by deno that they are now part of the standards committee. Its been a long time since WebSQL was originally proposed, maybe the web committees are ready for this now?

ok, do we need a poll?

I and @bartlomieju are supportive of native SQLite bindings. We've already discussed this internally multiple times and ended up #14627 (comment) but there is enough community feedback to assume that this is wanted.

I'm going to open it again for discussion. - @littledivy

if this is the case, is there any way to measure community desire for this feature? Is it just number of reactions/comments on this issue? Should there be a discussion poll?

Apologies for the long, long post. I find sqlite very useful and would love to see it in deno.

@kitsonk
Copy link
Contributor

kitsonk commented Aug 9, 2022

Deno.run isnt a web standard, and neither is Deno.dlopen, they are both just two very useful apis.

Correct. There are a few distinctions here:

  • They are fairly low level, primitive functions/methods.
  • They are analogues to binding to the operating system.
  • There is not a suitable web standard to provide the functionality.
  • You cannot provide the functionality any other way but exposing an API in the isolate. They cannot be user loadable (technically they could be but they would have to call too Rust and those calls to Rust would need to be exposed all the time and then the "usable" API user loadable).
  • The are located in the Deno namespace, to make it clear they are platform specific and non-standard, and ensure a developer realises they are limiting code portability by using them (or have to have some other approach).

The existing wasm sqlite integration builds on top of these bindings, even the FFI would go through these bindings. Keeping the bindings low level and as limited as possible should be the goal here.

One more followup here, the last 10% of the developer community will always still have the option to roll their own sqlite ffi lib or wasm lib if they want some very specific sqlite features that deno doesnt provide standard.

Why are not those solutions acceptable for everyone? Especially if the FFI solution, as it is expected to be, will be similar performance to a fully native solution. It becomes totally opt-in, specific to users workloads.

So, I understand this argument, but at the same time, the runtime is already fractured, and its a good thing.

It isn't a good thing. History has demonstrated it isn't. Part of Deno's participation WinterCG is to help ensure that server runtimes can move towards a standards based approach for things that only apply to server runtimes. Almost every challenge we face in adoption of Deno can be tied back to challenges working in the larger eco-system and people's expectations that "well it was ESM code so it should just work".

It seems like there are less hard rules about its design, and instead it just picks and chooses what works well.

Which is one of the things we think is different about Deno. It is more intentional about its design, advocating for a standards based approach.

I believe I remember a blog post by deno that they are now part of the standards committee.

There are several standards committees. TC39 governs ECMAScript, the language syntax and behaviour. W3C runs the WHATWG, which produces the higher order web API standards, like IndexedDB. The WinterCG is the server side focused working group. Deno is a member of TC39 and WinterCG. Deno participates in the WHATWG on specific proposals.

There were valid reasons why WebSQL got killed off, mostly that it was too coupled to the implementation. It is a good goal of the WHATWG to ensure that APIs are not coupled to closely to an implementation, because they should be more generalised.

What is needed in my opinion is straight forward key value storage that isn't IndexedDB and has a modern API that works well in the sandboxed and security model that works in browsers. That wouldn't be SQLite either. A few years ago Google tried to advocate for KVStorage but abandoned it.

I personally wouldn't want to advocate for something that I don't believe would be good for the web platform. SQLite is popular, SQLite is good, but baking the bindings directly into the runtime like the way Bun has means it becomes only a server side solution. A good solution in this space would be able to work well in a browser or in a server runtime, which means it needs to work well in the browser security model (supporting origin based quota management) and other things which are not instrinsic to SQLite. SQLite, like a lot of persistence solutions, is very workload specific. Why not Redis for example?

Deno should focus on ensuring the low-level APIs are available and that people can create libraries that perform well across the spectrum, while trying to be as conservative about exposing APIs for everyone, especially ones that are not part of the web platform.

@ghost
Copy link

ghost commented Aug 18, 2022

Hello - just want to chime in with some opinions from the point of view of one common developer:

  • If SQLite is baked into Deno or not doesn't matter much to me.
    • I'm already in the habit of finding third party packages to use [0].
    • Most likely I'd use a third party package anyway since I want as nice of a developer experience as possible and not just a basic SQLite implementation. (I'm building one right now to expand on denodrivers/sqlite3!)
    • The thing I'd be concerned about if it's baked in is: Are my Deno-compiled binaries going to be bigger because of it? (I doubt it.)
  • I just want a fast, native SQLite for server. I would have liked WebSQL for the browser.
  • WebSQL being removed from browsers was a travesty IMO. No common developer cares about ivory-tower standards of having multiple implementations. That's why the vast majority of us build for Chrome and call it a day.
    • If having "one implementation" of something were actually a problem in the real world, it would be a problem for every Linux distro since they all use the same kernel.
  • IndexedDB sucks and I don't care if it ever gets implemented. I'll never use it.
  • The wasm version of SQLite is a huge hack IMO, I don't trust it and I don't want to deal with all the complexity surrounding it.
    • I would never want to ship a huge blob of wasm for an online browser based web app.
    • I don't even know where to look to find out differences between the wasm version and the native version.
    • For a desktop app, I'd want the native version there too because I'd want to do stuff like WAL, WAL2, .backup, etc.

These are my opinions. No offense meant to anyone. Thanks!

[0] - Speaking of finding third party modules - I wish they would switch back to displaying the number of stars over at deno.land/x. As it stands today, the only place you can see the number of stars is when you search for a third party module, NOT when you look at the details page for it. Without the stars on the details page, I have to keep going back to the search page to compare 2 packages for popularity (or do the extra step of opening the Github pages). I understand this is all a work in progress though. Thanks again!

@tracker1
Copy link
Author

tracker1 commented Aug 23, 2022

Just chiming in, since I created the original issue. I still think a native option would be best for a lot of use cases. Deno.openSqlite3 might be the way to go. Not sure if sqlite3 is still in the runtime under the covers. I don't think IndexedDB is all that good at all, and would still need some overlay to present a better developer experience.

That said, when I made the request was before FFI was really even an option, and it may be good enough at this point, while native would still be faster. In the end, the ability to have multiple processes working against the same db file would still be a real nice to have (don't think the wasm will ever be able to do this). Even then, fracturing isn't something I'm so worried about, as there isn't a single standard Sqlite client interface as it stands, and even with the exteral modules, there still isn't and won't be.

As to the runtime size, if the library itself is already baked in, exposing another interface to it would be negligible in terms of additional size. In the end, I think the FFI implementation is probably good enough, but in the box would be better.

edit: Also, in the box would allow compliance with the existing read/write limitations while not opening a gaping hole in the security model for FFI permissions.

@andykais
Copy link

I personally wouldn't want to advocate for something that I don't believe would be good for the web platform. SQLite is popular, SQLite is good, but baking the bindings directly into the runtime like the way Bun has means it becomes only a server side solution. A good solution in this space would be able to work well in a browser or in a server runtime, which means it needs to work well in the browser security model (supporting origin based quota management) and other things which are not instrinsic to SQLite. SQLite, like a lot of persistence solutions, is very workload specific. Why not Redis for example?

I still think that its fair to say that deno has server-specific apis. Deno is being built around server-side workflows. Sure its possible that the deno team plans to support more interesting platforms in the future, but that shouldnt discount how important these are. What about Deno.core.ops? Deno.serve? Deno.dlopen? One could just as easily say 'why include an http server? Theres already Deno.listen.

I will grant you that SQLite is not a one-size-fits-all solution. There will be plenty of real world apps that need more powerful databases, like elasticsearch, redis, postgres, mongo, etc. We still need drivers for those. The case for native sqlite however, comes down to the fact that

A) rusqlite is already bundled with deno, interfaces are the only thing that need to change
B) sqlite is the best database to solve the case of small, enclosed systems (which for myself and I believe many others, is currently deno's bread and butter)
C) deno generally has great ergonomics, but there are not ergonomic solutions to libraries including native components. In the case of WASM, sqlite is missing many features like WAL mode (due to mmap requirements), and using FFI can be good enough, but there is no way to package binaries with your library anywhere, at best you can rely on them existing in your system already, or fetching them at runtime. These are not great ergonomics, and even solving these feels roundabout when the actual implementation already lives inside the deno runtime.
D) I still believe we will get the best possible speed out of a hand crafted interface from the deno team

@dsherret
Copy link
Member

dsherret commented Aug 30, 2022

rusqlite currently being in the binary is an implementation detail. It currently being in the binary is not a reason to expose this as part of Deno's runtime API IMO because it's possible we might remove it for a better solution/api in the future (though unlikely).

I agree with Kitson's comments above.

@littledivy
Copy link
Member

littledivy commented Sep 26, 2022

FYI https://github.com/denodrivers/sqlite3 is currently looking good for high performance needs, almost on par with C. I'm fairly confident that a native SQLite won't be faster than this.

@andykais
Copy link

Imo at this point ffi sqlite speed is pretty darn good, but what we are lacking here is a deno mechanism for delivering native binaries (or even compiling them some day). Currently, deno implements npm's binary retrieval mechanism, which solves a lot of these issues for npm modules, but not for deno modules. If I want to use https://deno.land/x/sqlite3 for instance, I need to --allow-net, --allow-read, --allow-write in addition to --allow-ffi (some may even use --allow-env=HOME to find the cache directory), and there isnt a great way to allow ffi just for that specific module. Heck, the deno sqlite3 docs simply suggest using --allow-all.

I think the general explanation here is that "if youre going to allow native code, youre breaking the security sandbox anyways". I think there should be a better way to say "Im allowing this bit of untrusted code to run" rather than just opening the floodgates to anything whenever we use ffi.

@drag0n-app
Copy link

https://docs.deno.com/kv/manual/on_deploy
As know from the deno.kv documentation that deno has an internal implementation of sqlite.
Could this be re-considered expose bindings directly on the Deno namespace?
@bartlomieju @lucacasonato

@sant123
Copy link

sant123 commented Jul 10, 2024

nodejs/node#53752

🤔

@tracker1
Copy link
Author

With node:sqlite landing in Node.js, it's probably time to re-evaluate the stance on this one...

https://nodejs.org/docs/latest/api/sqlite.html

@andykais
Copy link

Deno 1.45.3
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> import * as sqlite from 'node:sqlite'
Uncaught TypeError: Unknown built-in "node:" module: sqlite
    at async <anonymous>:1:37

I had to try 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion this topic needs further discussion to determine what action to take suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

9 participants