Add an optional options argument to Database constructor

[rhashimoto]

This replaces sqlite3_open with sqlite3_open_v2 and adds an optional options object argument to the Database constructor. The options object has 3 optional properties that match up with the sqlite3_open_v2 function:

    /**
     * @typedef {Object} Database.Options
     * @property {string} [filename] Database filename. Can be ":memory:"
     * to bypass the filesystem but "export" method should not be used.
     * If unset, a random name will be generated.
     * @property {number} [flags] Any combination of SQLite open flags. See
     * https://www.sqlite.org/c3ref/c_open_autoproxy.html (default:
     * SQLITE3_OPEN_CREATE | SQLITE3_OPEN_READWRITE).
     * @property {string} [vfs] VFS name. If specified, an initialization
     * array cannot be provided to the constructor and the "export" method
     * should not be used.
     */

Providing the ability to select a VFS is prep for #447.

[rhashimoto]

Looks like the build failed due to some sort of network problem. I don't think this is related to my code; it's after the tests succeeded. Is there any way to retry it?

[lovasoa]

I relaunched the build.

On the PR itself, I'm not sure the design is exactly right. Adding a second argument that makes the first argument illegal in some cases, but not all, referring to "file names" and ":memory:" even when everything is in memory, and referring to other file-related things is a little confusing. Also, there is no reason to disable export with other VFS, right ? The VFS can always be used to export the database as bytes.

And how would this PR articulate with the actual VFS creation ?

What I think would be more developer-friendly would be a design that would allow writing code like

const vfs = new IndexedDBVFS({ ...my_options });
const db = Database.with_vfs(vfs);

[rhashimoto]

Thanks for retrying the build!

I think we want the ability to specify the filename for a database; it's not a property of the VFS. And why not provide access to all the sqlite3_open_v2 arguments? How about this:

// This is outside the project domain.
const vfs = new IndexedDbVFS(whatever);

// This would call sqlite3_vfs_register.
Database.register_vfs("aName", vfs);

// Opening a database references VFS by name (not object).
// filename is required, flags is optional.
const db = Database.with_vfs("aName", filename, flags);

And how would I implement this? Can I add a secret unsupported argument to the Database constructor to create an instance for Database.with_vfs, or should I create an object without the constructor and manipulate its prototype?

Somewhat off-topic, does the ES5 code style still make sense? The optimized builds go through the Closure compiler anyway and couldn't we presume that developers using debug builds would be working with something a bit more recent?

[lovasoa]

I still don't think "filenames" do not make sense. The backend doesn't need to have (and in most cases won't have) a notion of file. Same for registering the VFS. This is an implementation detail that the user shouldn't have to worry about. For flags, they don't make sense for most VFS on the web, but if they do for one in particular, they can take care of it.

Maybe I shouldn't call the javascript interface a "VFS", maybe just "backend". If there is a backend that is backed by actual files (on node.js, for instance), then it can offer the options to set a file name, file permissions, and so on.

For the code style, yes, I agree working with modern js would be more pleasant ! We would have to make sure the result is still compiled to es5, though.

[lovasoa]

About VFS registration, I think it should be handled by the library directly, not the user. The backend interface we define should expose a VFS, and sql.js will handle registering the VFS if it's absent when creating the database.

[rhashimoto]

I don't see filename as an implementation detail. I see it as part of the SQLite API. And the same with VFS.

The Database class is easy to use and that's a valuable thing. But as a developer, I want it to give me access to SQLite, not hide it from me. I feel that if you hide too much behind an abstraction, developers are going to want you to break open the abstraction. It's pretty easy to layer on more abstraction if that's what you want. That's part of what I was getting at with my #377 comment.

Minimizing the abstraction also makes things easier for you, the maintainer. There's a lot less thought required if you're just exposing SQLite concepts and not inventing new ones.

What's the naming convention? I see create_function, but also getRowsModified, iterateStatements, etc. Is there a heuristic for choosing with_vfs?

[lovasoa]

Yes, the SQLite API shouldn't be inaccessible to the developer, it should just not be directly visible in the cases where it is not relevant. I think the largest use case for this work is going to be the indexedDB backend, for which the notions of filename or file permissions don't make sense. We will simply let those who want to interact with sqlite at a lower level implement the Backend interface. An interface implementation could still wrap another one.

About the naming convention: yes, we haven't been consistent in the past, when accepting the PR for create_function. I think we should go with camelCase.

[rhashimoto]

I don't agree, but it's certainly your prerogative to define the API. Given that this API is not the one that I would be using, perhaps it makes more sense for someone else to implement it.