2shady4u / godot-sqlite

GDExtension wrapper for SQLite (Godot 4.x+)
MIT License
947 stars 82 forks source link

Verbosity level QUIET isn't really quiet #86

Closed Cryptoclysm closed 8 months ago

Cryptoclysm commented 2 years ago

Environment:

Issue description:

As it stands, the query_with_bindings method will still print error messages, even when 'QUIET' is enabled. This is a problem, because there is no way to detect database locked without generating an error message that gets logged to the Godot console.

Here is the code responsible, in the master branch, gdsqlite.cpp, lines 197 to 202. It happens in a few other places too.

if (rc != SQLITE_OK)
{
        GODOT_LOG(2, " --> SQL error: " + error_message)
        sqlite3_finalize(stmt);
        return false;
}

There is no check for verbosity, as it stands.

I can see it being useful to display some errors, even when a vebosity level of QUIET is enabled, but ERROR: --> SQL error: database is locked happens as a normal part of the operation of my application, while I wait for the lock to be released, so ideally this would not be printed with QUIET enabled.

Steps to reproduce:

Hopefully the description is enough to illustrate the problem. This should work for displaying a 'database is locked' error, but may depend on the OS:

Using a program like DB Browser (SQLite) or similar, create an empty database 'db.db'. Keep the DB open in the DB Browser application. Then run any query on that database, using the code below:

db1 = SQLite.new()
db1.path = "db.db"
db1.verbosity_level = 0

db1.open_db()
db1.query("SELECT type,name,sql,tbl_name FROM "main".sqlite_master;")
db1.close_db()

This should show the ERROR: --> SQL error: database is locked error.

2shady4u commented 2 years ago

Hello @Cryptoclysm

I don't think hiding ALL the errors when setting the verbosity_level is QUIET is a good choice as errors should be shown regarding of the verbosity's level.

Another option would be to only show 'critical' errors by checking the error code (SQLITE_CORRUPT would always be printed, while SQLITE_LOCKED would only result in a print if the verbosity_level is higher than QUIET), but as there are a lot of error codes this seems to be difficult job.

If I may ask, what is the actual use-case you are facing where you are trying to modify a locked database during normal operation?

Cryptoclysm commented 2 years ago

My use case here is that I am reading from an SQLite database that is written to by another application. The other application completes Web Requests and writes them to the SQLite database. So on occasion, the Godot application will encounter a locked database, and have to wait to retry again when it's no longer locked.

So it would be very useful in my case to be able to omit the log message for a locked database - it's resulting in some very long log files, and logging in Godot can hit performance fairly majorly.

Cryptoclysm commented 2 years ago

@2shady4u I've been thinking about this issue more recently. I think the real solution to my issue would be to have access to the busy_timout variable in SQLite.

That way concurrent database access is possible without triggering a SQLITE_LOCKED exception at all. It would also allow Litestream to be used for live replication of data in a server application.

As it stands I don't think there is a way to do this within Godot-SQLite, could this be added in the future?

2shady4u commented 2 years ago

Hi @Cryptoclysm,

I've been extremely busy lately with stuff happening in the real world so I haven't had the chance yet to ponder about the correct way to implement your request. You should already have access to the busy_timeout variable by querying the following pragma:

db.query("PRAGMA busy_timeout;")
print("busy_timeout is: {0}".format([db.query_result[0]["busy_timeout"]]))

However, I fail to see how increasing the busy_timeout fixes the issue? If I'm correct, the chance of the issue happening would be much much less, but wouldn't you occasionally still get SQLITE_LOCKED exceptions?

Cryptoclysm commented 2 years ago

@2shady4u You're correct - this won't fix the issue completely, but will reduce the chance of it happening dramatically. Thanks for the info on accessing busy_timeout. Please do take your time, I appreciate your work on this library massively, it's great that Godot can be so capable without ever leaving gd script!

2shady4u commented 8 months ago

Closing this issue as the latest version of this plugin (as found on the master-branch) should behave quietly whenever the verbosity level is set to QUIET.