Change exec(...) to return column names for empty result sets #388
Conversation
|
This would be a breaking change. This requires changes to the documentation and to the tests. |
|
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. |
|
It's worth noting, that the type of code this might break is code that runs on a single SELECT |
|
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 The documentation to update is the jsdoc on top of the function. |
486b3bb
to
289632d
289632d
to
4567872
|
Okay, I've fixed my patch & written tests & docs now. |
|
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, this improves product-integration -- can copy-pasted sql.js into custom rollups // 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"); |
|
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;
} |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

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.