Change exec(...) to return column names for empty result sets

[frankier]

Currently exec(...) will not put anything in its results array for a SELECT query which returns no results. This is not ideal. Say I run 3 SELECT queries, and 1 returns an empty result set. How do I know which query isn't included in the results array? With this change, you can always tell what the size of the results array will be just by inspecting the query. I have also changed getColumnNames(...) to work for empty result sets by using sqlite3_column_count instead of sqlite3_data_count.

[lovasoa]

This would be a breaking change. This requires changes to the documentation and to the tests.

[frankier]

Well the previous documentation didn't specify the old behaviour, so it's not clear where to make change. A note could be added to the effect of "before $VERSION this did $X now it does $Y" but I don't know what $VERSION would be since I don't know what the release process/versioning protocol is -- so I think someone who knows the value of $VERSION might have to do that? As for tests, it looks like the CI has failed but it just says "Error: no such table: test". I'm not sure --- but I don't think this is a consequence of my commit --- is it possible to rerun the CI to check, and how? In case I can get the CI working, of course I could add a regression test.

[frankier]

It's worth noting, that the type of code this might break is code that runs on a single SELECT res = exec("SELECT ...") and then checks that res.length === 0 to see if there are no results. It doesn't seem likely that an exec involving more than one SELECT is likely to rely on the current behaviour for the reasons underlined in my initial report.

[lovasoa]

I do think that the broken tests are a consequence of your changes. I re-launched the tests to make sure. You can run the tests locally to make sure they pass before pushing a commit.

The currently released version is 1.2.2. I think this change would require us to switch to 2.0, since this is a breaking API change. https://github.com/sql-js/sql.js/releases
We should probably think about the other breaking changes we want to make so that we can release them all at once.

The documentation to update is the jsdoc on top of the function.

[frankier]

Okay, I've fixed my patch & written tests & docs now.

[kaizhu256]

here's another breaking changes to consider for v2.0:

if the web-worker-code is to be merged into mainline, then require explicit query,
e.g. ?initSqlJsWorker=1 to activate.

this improves product-integration -- can copy-pasted sql.js into custom rollups
w/o interfering with user-defined worker-initialization code, e.g.:

// rollup-file with sql.js - explicitly run default web-worker-code
var worker = new Worker("/dist/rollup.js?initSqlJsWorker=1");

// rollup-file with sql.js - do not run default web-worker-code
// (use custom worker-initialization in rollup like comlink, etc..)
var worker = new Worker("/dist/rollup.js");

[lovasoa]

Now that #412 and #429 have been merged, one can achieve the same results as the proposed updated exec, in a few lines of code, without needing a breaking change to the API :

function exec(db, sql, params) {
  const results = [];
  for (const statement of db.iterateStatements(sql)) {
    statement.bind(params);
    const columns = statement.getColumnNames();
    const values = [];
    while (statement.step()) values.push(statement.get());
    results.push({ columns, values });
  }
  return results;
}

[lovasoa]

See https://sql.js.org/documentation/Database.html#%5B%22iterateStatements%22%5D