Skip to content
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

[11.x] Support named in-memory SQLite connections #53635

Merged
merged 5 commits into from
Nov 25, 2024

Conversation

stancl
Copy link
Contributor

@stancl stancl commented Nov 22, 2024

SQLite supports named in-memory databases. The benefit of named connections is that you can essentially "reconnect" to the same memory region even without passing a PDO instance around. Only the DSN needs to be the same.

Here's an example:

<?php

function ensure_table_exists(PDO $conn) {
    if (! $conn->query('SELECT name FROM sqlite_master WHERE type="table" AND name="my_table"')->fetch()) {
        $conn->exec('CREATE TABLE my_table (name text)');
    }
}

function foo(string $dsn) {
    $conn = new PDO($dsn);
    ensure_table_exists($conn);

    $conn->exec('INSERT INTO my_table VALUES ("foo")');
    var_dump(array_map(fn ($res) => $res[0], $conn->query('SELECT * from my_table')->fetchAll())); // foo

    return $conn;
}

function bar(string $dsn) {
    $conn = new PDO($dsn);
    ensure_table_exists($conn);

    $conn->exec('INSERT INTO my_table VALUES ("bar")');
    var_dump(array_map(fn ($res) => $res[0], $conn->query('SELECT * from my_table')->fetchAll())); // foo + bar

    return $conn;
}

$dsn = "sqlite:file:mydb?mode=memory&cache=shared";
// $dsn = 'sqlite::memory:';

$_ = foo($dsn);
bar($dsn);

It shows how you can connect to the same database, without passing the PDO instance directly, as long as the DSN string is identical. foo() will log foo while bar() will log foo and bar. The only condition here is that the connection is kept alive somehow.

If you swap the $dsn definitions, both foo() and bar() will create their own database when the PDO connection is created (and free it once $conn goes out of scope). Additionally, if you remove the $_ =, the memory of the database will be freed as there's no longer any open connection, so it'll behave identically to :memory:.

Practical use case

This lets me support :memory: in my tenancy package for testing tenant databases, providing a nice speedup. During the tenant creation process there's a lot of logic that essentially keeps connecting to and disconnecting from the tenant DB. Users may also switch to the central context using tenancy()->end() or tenancy()->central(closure) which closes the tenant connection.

With named databases, I only need to keep a connection to the tenant DB alive somewhere (literally just register_shutdown_function(fn () => $conn) works) and then the package can seamlessly connect to and disconnect from tenant databases that live entirely in process memory and don't need any special cleanup.

Code style

I could just check for mode=memory but that could in theory be part of a real filename. Don't think it'd make anything vulnerable if the realpath() check got skipped, but I went with ?mode || &mode just in case since it's a little more restrictive.

The DSN syntax for named in-memory databases is just mode=memory in URI format typically paired with cache=shared. The order being arbitrary.

I also tried to respect the 3 char rule in the comment but couldn't think of anything that'd fit here better, so the last line is an extra character shorter.

@stancl
Copy link
Contributor Author

stancl commented Nov 23, 2024

If you want I can also add a couple of tests — might prevent some future regressions since :memory: tends to be hardcoded in most code.

Looks like these are the only relevant tests of the connector itself:

public function testSQLiteMemoryDatabasesMayBeConnectedTo()
{
$dsn = 'sqlite::memory:';
$config = ['database' => ':memory:'];
$connector = $this->getMockBuilder(SQLiteConnector::class)->onlyMethods(['createConnection', 'getOptions'])->getMock();
$connection = m::mock(stdClass::class);
$connector->expects($this->once())->method('getOptions')->with($this->equalTo($config))->willReturn(['options']);
$connector->expects($this->once())->method('createConnection')->with($this->equalTo($dsn), $this->equalTo($config), $this->equalTo(['options']))->willReturn($connection);
$result = $connector->connect($config);
$this->assertSame($result, $connection);
}
public function testSQLiteFileDatabasesMayBeConnectedTo()
{
$dsn = 'sqlite:'.__DIR__;
$config = ['database' => __DIR__];
$connector = $this->getMockBuilder(SQLiteConnector::class)->onlyMethods(['createConnection', 'getOptions'])->getMock();
$connection = m::mock(stdClass::class);
$connector->expects($this->once())->method('getOptions')->with($this->equalTo($config))->willReturn(['options']);
$connector->expects($this->once())->method('createConnection')->with($this->equalTo($dsn), $this->equalTo($config), $this->equalTo(['options']))->willReturn($connection);
$result = $connector->connect($config);
$this->assertSame($result, $connection);
}

So I could just add a similar test for named databases as there is for in memory databases. There's probably no need to test that "named connections work" since that's PDO's responsibility.

Regarding hardcoded :memory: strings, these are the only such spots in the codebase (excluding tests/):

src/Illuminate/Testing/Concerns/TestDatabases.php|150 col 28| if ($database !== ':memory:') {
src/Illuminate/Database/Schema/SQLiteBuilder.php|93 col 55| if ($this->connection->getDatabaseName() !== ':memory:') {
src/Illuminate/Database/Schema/SqliteSchemaState.php|63 col 55| if ($this->connection->getDatabaseName() === ':memory:') {
src/Illuminate/Database/Connectors/SQLiteConnector.php|24 col 38| if ($config['database'] === ':memory:' || str_contains($config['database'], 'mode=memory')) {
src/Illuminate/Foundation/Testing/RefreshDatabase.php|39 col 70| return config("database.connections.$default.database") === ':memory:';
  • TestDatabases and RefreshDatabase can imo be ignored since the methods can be easily overridden by the user as needed (in my tests I didn't even find this to be necessary)
  • I could perhaps update this bit:
    public function dropAllTables()
    {
    if ($this->connection->getDatabaseName() !== ':memory:') {
    return $this->refreshDatabaseFile();
    }
  • Same thing here:
    public function load($path)
    {
    if ($this->connection->getDatabaseName() === ':memory:') {
    $this->connection->getPdo()->exec($this->files->get($path));
    return;
    }

So let me know if I should add any specific tests. I'll probably just push changes to the two snippets above and add the connector mock test for named in-memory DBs.

@stancl
Copy link
Contributor Author

stancl commented Nov 24, 2024

I've added the test now and confirmed that if I revert my patch to SQLiteConnector, it fails with:

1) Illuminate\Tests\Database\DatabaseConnectorTest::testSQLiteNamedMemoryDatabasesMayBeConnectedTo
Illuminate\Database\SQLiteDatabaseDoesNotExistException: Database file at path [file:mydb?mode=memory&cache=shared] does not exist. Ensure this is an absolute path to the database.

Now I could go over a few spots in the tests and make sure all tests that test the behavior I've extended here are also tested with named in-memory databases but I don't think that should be necessary. If you think those changes should be made I don't have a problem with making them though.

@taylorotwell taylorotwell merged commit a2bcf35 into laravel:11.x Nov 25, 2024
38 checks passed
@stancl
Copy link
Contributor Author

stancl commented Nov 25, 2024

Thanks!

crynobone added a commit to orchestral/testbench-core that referenced this pull request Dec 5, 2024
PR: laravel/framework#53635

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
crynobone added a commit to orchestral/testbench-core that referenced this pull request Dec 5, 2024
* [9.x] Add support for named in-memory SQLite connections

PR: laravel/framework#53635

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>

* wip

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>

---------

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
crynobone added a commit to orchestral/testbench-core that referenced this pull request Dec 5, 2024
* [9.x] Add support for named in-memory SQLite connections

PR: laravel/framework#53635

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>

* wip

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>

---------

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants