MDL-88218: Add experimental SQLite support for Moodle WASM environments (4.5)#4
Open
erseco wants to merge 3 commits into
Open
MDL-88218: Add experimental SQLite support for Moodle WASM environments (4.5)#4erseco wants to merge 3 commits into
erseco wants to merge 3 commits into
Conversation
Moodle 4.5 still ships the legacy SQLite PDO driver (removed in 5.0+, which is why the 5.0/5.1/main branches re-add a modernised one). That legacy driver was left behind as the DML/DDL base classes evolved, so it can no longer install a site on 4.5. This makes it usable again (experimental; intended for WASM / demo / testing environments, like MDL-88218 on the newer branches). Fixes: - sqlite_sql_generator::getCreateTempTableSQL(): 4.5's sql_generator declares this method abstract, so the generator was left abstract and could not be instantiated. Implement it (CREATE TEMPORARY TABLE, mirroring the mysql generator). - sqlite_sql_generator::__construct(): forward the $temptables argument that get_manager() passes to the parent constructor; otherwise $this->temptables is null and every table create fails with "is_temptable() on null". - sqlite3_pdo_moodle_database::connect(): 4.5 expects each driver to create its temptables controller in connect(); initialise moodle_temptables so the DDL generator can query temp tables during install. - sqlite3_pdo_moodle_database::fetch_columns(): declared ": array" but returned false when a table's CREATE TABLE could not be found -> TypeError on PHP 8. Return an empty array instead. - pdo_moodle_database::get_records_sql()/get_records_select(): return an empty array (not false) when there are no rows, matching the documented contract; 4.5 core (e.g. license install) calls array_map() on the result. - admin/environment.xml: declare the sqlite database VENDOR in the 4.5 block so install_database.php's environment check recognises it. With these, a full CLI install (admin/cli/install_database.php) completes on SQLite and the site serves.
database_column_info::__construct() reads its fields via object property access, so the SQLite driver's fetch_columns() must pass an object like the pgsql and mysqli drivers do. Passing the raw array left every property (including ->name) null, which broke callers reading $column->name (e.g. theme_snap's events query produced 'SELECT e., e., ...' and a syntax error).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion to #1 (main), #2 (5.1) and #3 (5.0). Moodle 4.5 still ships the
legacy SQLite PDO driver (
lib/dml/sqlite3_pdo_moodle_database.php+lib/ddl/sqlite_sql_generator.php) that was removed on 5.0+ — which is why thenewer branches add a modernised driver, while 4.5 only needs the existing one
repaired.
That legacy driver was left behind as the DML/DDL base classes evolved, so on
current 4.5 it can no longer install a site (
admin/cli/install_database.phpfails). This PR makes it work again (experimental; WASM / demo / testing only).
Fixes
lib/ddl/sqlite_sql_generator.phpsql_generatordeclaresgetCreateTempTableSQL()abstract; the generator omits it, so the class is abstract and cannot be instantiated (silent exit 255).getCreateTempTableSQL()(CREATE TEMPORARY TABLE, mirroringmysql_sql_generator).lib/ddl/sqlite_sql_generator.php__construct($mdb)drops the$temptablesargument thatget_manager()passes →$this->temptablesis null →Call to a member function is_temptable() on null.$temptablesto the parent constructor.lib/dml/sqlite3_pdo_moodle_database.phpconnect(); this driver never does →$this->temptablesnull.connect()to initialisemoodle_temptables.lib/dml/sqlite3_pdo_moodle_database.phpfetch_columns(): arrayreturnsfalsewhen a table's CREATE TABLE isn't found →TypeErroron PHP 8.lib/dml/pdo_moodle_database.phpget_records_sql()/get_records_select()returnfalseon an empty result, but the documented contract (and callers such as license install, which doesarray_map()) expect an empty array.array()when there are no rows.admin/environment.xml<MOODLE version="4.5">block does not list the sqlite database vendor, soinstall_database.php's environment check rejects it.<VENDOR name="sqlite" version="3.0" />in the 4.5 block.Verification
Built Moodle 4.5.12 with these changes on top of an
erseco/alpine-moodleimage and ran a full CLI install on SQLite:
admin/cli/install_database.phpcompletes (496 tables, admin + guest users created).moosh role-listworks.php -lclean on all changed files;environment.xmlis well-formed.Notes
(4 files, +40/−5).
pdo_moodle_database.phpis the shared PDO base, but SQLite is its onlyconsumer here; the empty-array change also matches the method's own docblock.