Fixed #24018 -- Allowed setting pragma options on SQLite.

[linville]

Adds support to the SQLite backend for the init_command option so user can set PRAGMA options when setting up the connection.

SQLite doesn't natively support the init_command keyword (such as MySQL does), the command is just run right after the connection is established.

Ticket #24018 in Trac

[github-actions]

Hello @linville! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

[felixxm]

@linville Thanks for the patch 👍 I can see one issue with this approach, we don't inform users when sth goes wrong.

[linville]

@linville Thanks for the patch 👍 I can see one issue with this approach, we don't inform users when sth goes wrong.

Thanks for reviewing! Could you expand on "when sth goes wrong", not sure I'm following? Would this be if the user uses invalid pragma/syntax in the init_command?

[felixxm]

Thanks for reviewing! Could you expand on "when sth goes wrong", not sure I'm following? Would this be if the user uses invalid pragma/syntax in the init_command?

As in the ticket description, PRAGMA journal_mode=wal; return wal on success and delete/memory when it fails. We don't validate the output so a user is not aware that journal_mode is unchanged. Maybe that's fine.

See also Carlton's comment.

[linville]

As in the ticket description, PRAGMA journal_mode=wal; return wal on success and delete/memory when it fails. We don't validate the output so a user is not aware that journal_mode is unchanged. Maybe that's fine.

Looking at the MySQL backend behavior for init_command, if something goes wrong the django.db.utils.ProgrammingError is raised with the SQL statement which would be useful to alert the user to errors. Having the sqlite backend implementation of init_command behave similar to the MySQL one would certainly be reasonable (though I suppose the opposite could be argued since it isn't Django or even the MySQL python module doing the validation but rather MySQL itself).

Thinking about approaches for this: whether a value is returned from a pragma statement varies on the pragma, some will return a value (e.g.: journal_mode) and some will not (e.g.: cache_size). It looks like an implementation of this would involve running two cursor executes for each pragma being set by the init_command: first execute the pragma setting statement and then then running another cursor execute to get the pragma's value and check that it matches what was assigned (this code would have to split multiple pragma set statements in the init_command and also extract the pragma names, but the syntax is simple so this shouldn't be too complex).

If we'd like to do validation, I'd be happy to take a shot at an implementation of the above approach if it seems reasonable.

[bcail]

@linville @felixxm Is there anything I can do to help move this forward?

[bcail]

I'm not sure we'd need to validate the result of the PRAGMA command. We don't verify the PRAGMA statements that django always runs. The user could also manually verify the value if desired.

[anze3db]

I think validating every statement in the init_command isn't practical because we don't control what the users actually do. The way that the current PR is implemented there is nothing stopping users from writing CREATE TABLE/INSERT/UPDATE/... statements in the init_command.

If we do want validation, then I think a better approach would be to expose the pragma settings that we want to support and validate through DATABASE OPTIONS e.g. DATABASE["default"]["OPTIONS"]["journal_mode"].

To me personally either approach works, I'd just really like to have this feature land in 5.1! Happy to help out in any way I can to get this done.

[felixxm]

@linville Thanks for updates 👍 I left small suggestions.

[gcollazo]

Great work thanks!

Would it be possible/make sense to set or recommend (in docs) a "sensible default configuration" for using SQLite in the context of a web app?

Ruby on Rails being very opinionated, decided to set some defaults. To me this makes a lot sense since this is mostly the configuration I'm using in my web projects.

The only thing missing is setting the busy_timeout but I think they have implemented a connection retry and backoff mechanism to deal with that at the framework layer instead of at the SQLite layer.

DEFAULT_PRAGMAS = {
    "foreign_keys"        => true,
    "journal_mode"        => :wal,
    "synchronous"         => :normal,
    "mmap_size"           => 134217728, # 128 megabytes
    "journal_size_limit"  => 67108864, # 64 megabytes
    "cache_size"          => 2000
}

Here's a link to the RoR settings for SQLite and here's a great blog post that helps understand the configuration and performs some benchmarks.

[felixxm]

Would it be possible/make sense to set or recommend (in docs) a "sensible default configuration" for using SQLite in the context of a web app?

I don't think so. If we decided we needed more options we would set them automatically, as we already do:

django/django/db/backends/sqlite3/base.py

Lines 200 to 203 in c991602

	conn.execute("PRAGMA foreign_keys = ON")
	# The macOS bundled SQLite defaults legacy_alter_table ON, which
	# prevents atomic table renames.
	conn.execute("PRAGMA legacy_alter_table = OFF")

You can start a discussion on the Django Forum if you strongly believe that Django should set more options by default.

[linville]

@linville Thanks for updates 👍 I left small suggestions.

Thank you for the review! I believe (hope!) I've got the requested changes incorporated now.

[felixxm]

I noticed that it won't work for multiple PRAGMA statements:

sqlite3.Warning: You can only execute one statement at a time.

I think we should change it to the list. It's probably better for consistency with MySQL options to split this on ;.

[felixxm]

@linville Thanks 👍 Welcome aboard ⛵