-
-
Notifications
You must be signed in to change notification settings - Fork 411
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
Executing other queries while iterating through a SELECT statement #203
Comments
Actually no, SQLite3 can only execute one statement at a time. While iterating through a result set with The solution is to just retrieve all rows before updating them. for (let row of statements.fetch.all()) {
statements.migrate.run(1, row.id);
} |
@JoshuaWise Thanks for the follow-up, I'm under the impression that I did the same (acquire cursor, iterate + update) in PHP with pdo-sqlite extension long ago and it worked fine. My memory could be failing me though... |
@JoshuaWise Confirmed - it works with PHP & SQLite: DB('sqlite://iterate_update.db');
DB('CREATE TABLE IF NOT EXISTS "migration" ("done" INTEGER, "id" INTEGER PRIMARY KEY);');
for ($i = 0; $i <= 100; $i++) {
DB('INSERT OR IGNORE INTO "migration" ("done", "id") VALUES (0, ?);', $i);
}
var_dump(DB('SELECT COUNT(1) FROM "migration" WHERE "done" = ?;', 0)->fetch()); // 100
$rows = DB('SELECT * FROM "migration" WHERE "done" = ?;', 0); // acquires cursor
while ($row = $rows->fetch()) { // iterates and updates within cursor
DB('UPDATE "migration" SET "done" = ? WHERE "id" = ?;', 1, $row->id);
}
var_dump(DB('SELECT COUNT(1) FROM "migration" WHERE "done" = ?;', 0)->fetch()); // 0 So this must be something specific to either |
@alixaxel Hmmm, I've just reviewed the C API and it seems actually you're right. I seem to remember thinking it would be better to prevent other DB actions while iterating, but I can't remember the reasoning now! I'll think on this, and perhaps remove the restriction in a future version. My statement from before, "SQLite3 can only execute one statement at a time," does apply to parallel or asynchronous execution, but not to synchronous execution. |
It should be noted that this is already possible by using two separate database connections. However, it's not an ideal workaround because you wouldn't be able to put the two iterations in the same transaction. |
Please fix this whenever possible you have time and motivation, this is very basic for a dbs....., thank you! |
As of v5.4.0, you can now execute read-only statements ( "Why?", you might ask. Well, according to this official SQLite3 documentation, the behavior is undefined when attempting to do this. If you can do this in PHP and it works as you expect it to, you're only getting lucky—the behavior might change in any new release of SQLite3. This is not a style of operation that any application using SQLite3 should rely on. It's not safe.
|
@JoshuaWise Sorry for only following up now on this but I was on holidays with limited internet availability. Regarding your rationale and after reading the documentation, I would say that undefined != unsafe.
I would prefer being responsible enough to choose when I might need to execute a A simple, safe example would be updating a table not referenced by the current Again from the docs:
So writing to current or previous rows of the cursor is safe and well-defined. This is very useful IMO. The undefined outcome seems to only refer to rows written ahead of the cursor:
And the docs finish with the same warning/recommendation to developers:
It would be great if you would reconsider unblocking this behavior and leave isolation concerns to the users. As it is, it's making too many assumptions that will often times be wrong and limitative. |
Unless your situation is very bottlenecked by performance, there's almost certainly another way of accomplishing the same ultimate result, albeit less conveniently. Should |
You can just add an option on the lines of "use_unsafe" :true. with some sentences explaining the trade offs. Im strongly against that trend that I see more and more from people that creates software, browsers, libraries, editors, that kinda make it impossible for people to get what they want because of "by design" reasons. Please don't take me wrong, I really appreciate a lot you sharing your work, with 1 user or with thousands it does not matter, you taking the effort to share it, which means a lot to me personally. I also understand you concerned for good reasons. I think we should always be giving people the power to decide by themselves, even if in a few cases they don't understand they shooting themselves. The power you give people that understand what they doing by far exceeds these that are learning or make mistakes. |
@JoshuaWise I'd like to try to change your mind about this, so let me explain where I'm coming from. I see SQLite has a super-efficient heavy duty DB, specially useful for storing and operating on a large amount of records. I have several multi-GB SQLite DBs and I usually get better performance with it than with a conventional client-server RDBMS due to the lack of network overhead, etc. And Currently I'm working with a SQLite DB that contains a couple dozen million queued migrations: I'm taking a schemaless document-oriented DB that has a very inconsistent structure (with years of GIGO practice), casting that inconsistent structure to a more consistent one, and writing it back to SQLite with
Which might be unfeasible if the amount of available memory isn't enough to keep a copy of all the rowids to update. And even if that's possible, it makes writing consistent code more complex (what if halfway through, applying one specific migration in the other schemaless DB fails, etc).
Which is what I'm using to overcome this limitation. However, the query performance is not as efficient, since the query has to be constantly re-evaluated and on memory-mapped result set is stale after each execution. Besides those, I don't see any other viable possibility. Given that cursor based iteration is the usual way to approach larger-than-RAM result sets, I would argue that neither of those alternatives is really the best one. When dealing with large datasets (again, one of the areas where SQLite really shines) the first option will often not be possible, and the second will not be as fast. And neither of those are as convenient / straightforward / elegant as the idiomatic way of: db.exec('BEGIN;');
for (let row of statements.fetch.iterate()) {
statements.migrate.run(1, row.id);
}
db.exec('COMMIT;'); Which, according to the documentation, is not only safe but well-defined too (since we are mutating the data that was already consumed by the cursor) - I would also argue that 99% of the times you mutate data within a cursor, you are mutating the same record you're iterating on, and not prior or ahead records. If someone is reckless enough to mangle It seems to me that this design decision is preventing a technique that I see as a best-practice in favor of protecting users that adopt bad practices from their own mistakes. This is very limitative IMO, and since SQLite itself didn't chose to block that behavior, why should this package do that? To give you another example, in another multi-GB SQLite DB where I have nearly a billion records, I:
Which again, is totally safe and well-defined. I do this currently in Golang, without any problem. However, with I appreciate the work you put into this package regardless. :) |
I got same issue. I have two tables:
I want to iterate all records from "pages_html" table, analyse "content" field, get all links to companies from there and insert to "companies" table. But I can't do that, because "pages_html" doesn't fit in memory. I solved it ugly way, but it works for me - I created two databases and store one table in each db. |
I also tried to use two connections like suggested above:
This is part of code that shows how I tried it with two connections:
But I'm getting this error:
About undefined behavior. If sqlite developers allowed something with undefined behavior and even described it in docs, it's not because it was hard for the developers to restrict it, there is clear reason behind it. And by my opinion database's driver should not decide what features of database should be restricted. |
Please reconsider. They are some very good reasons to allow to iterate over a result from a table while updating another table. This is not an anti-pattern, and is the way to go. It is performant and elegant. |
Hi all, I had a crack at this today in #367 |
This is now possible in v7.0.0 via unsafe mode |
I don't know if this is already mentioned somewhere but I was hitting this problem as well and to get around it I just created two connections to the database. const reader = new Database("DB.sqlite3");
const writer = new Database("DB.sqlite3"); Then just prepare the writing statements with the writer connection and the reader statements with the reader connection. Simple solution and it works great! Although I was writing to a different table than I was reading from. Edit I now see that this was already mentioned. Sorry! |
I'm using the latest version of
better-sqlite3
and I have some simple code that looks like the following:Whenever I try to run it however, I always get the following error:
How can I update a row while iterating through it? Surely it must be possible?
The text was updated successfully, but these errors were encountered: