[10.x] Laravel 10x optional withSize for hasTable

[apspan]

In an effort to enhance the performance of SQLite tests and streamline the functionality of the SQLiteBuilder's getTables method, this pull request introduces an optimisation by making the withSize parameter optional. This change allows for a reduction in unnecessary queries, particularly for trivial actions where table size information is not needed.

Background:

Previously, the determination of whether to include table size in the results (withSize) was attempted through a direct query using compileDbstatExists(). This approach was fixed and did not account for cases where fetching table size was not required, leading to unnecessary execution of database queries.

Implementation Changes:

The new implementation introduces an optional parameter $withSize to the getTables method. This parameter allows callers to explicitly state whether table size information should be included (true), not included (false), or decided based on the existence of the DBSTAT virtual table (null). This is achieved through a match expression that either directly uses the $forceWithSize value if it's boolean or dynamically determines the value by attempting to query the database for the DBSTAT virtual table's existence. This approach significantly reduces unnecessary database interactions, especially in scenarios where table size information is irrelevant.

Benchmarks

I setup a simple benchmark check in DatabaseMigratorIntegrationTest just to get proof:

    public function testBenchmarks()
    {
        $this->migrator->run([__DIR__.'/migrations/one']);
        $this->migrator->run([__DIR__.'/migrations/two']);

        // init call to warm up the cache and get more realistic results
        $this->db->schema()->getTables();

        Benchmark::dd([
            'With default' => fn () => $this->db->schema()->getTables(), // 0.143 ms
            'With true' => fn () => $this->db->schema()->getTables(withSize: true), // 0.134 ms
            'With false' => fn () => $this->db->schema()->getTables(withSize: false), // 0.018 ms
        ], 20);
    }

Benefits to End Users:

Improved Performance: By avoiding unnecessary queries, the overall performance of interacting with SQLite databases, especially in testing environments, is improved.
Flexibility: Users now have the option to explicitly decide whether or not table size information is needed, allowing for more tailored and efficient database interactions.
Simplicity and Backward Compatibility: The change is implemented in a backward-compatible manner, ensuring that existing applications will not be affected. The simplicity of opting in or out of fetching table size information makes this enhancement seamlessly integrate into existing workflows.

Ensuring Non-breaking Change:

This update does not alter the default behaviour of the getTables method unless explicitly instructed by the user, ensuring no unexpected changes for existing users.
Comprehensive testing has been conducted to ensure that this change does not introduce any regressions or break any existing features.

Conclusion:

Making the withSize parameter optional in the SQLiteBuilder's getTables method is a significant optimisation that reduces unnecessary database queries, thereby speeding up SQLite tests and making web application development easier and more efficient. This change demonstrates a commitment to improving the developer experience without compromising the integrity of existing applications.

Existing test coverage should be sufficient to confirm that the existing behaviour is unchanged.

[GrahamCampbell]

This is not a bug fix, so is not suitable for backport to 10.x. Only 11.x should have this change.

[apspan]

This is not a bug fix, so is not suitable for backport to 10.x. Only 11.x should have this change.

Hey, @GrahamCampbell thanks for taking a look at my PR and for the feedback.

I understand where you're coming from with this seeming more like a fit for 11.x since it's not a bug fix in the traditional sense.

However, I want to point out that the change introduced in 10.44 has inadvertently affected our (and many others') testing speeds and, in a way, altered the behaviour we've come to expect. This optimisation I'm proposing isn't just about fixing bugs; it's about correcting a slowdown that impacts our CI efficiency significantly. It makes a real difference in how quickly we can run tests and streamline CI pipelines, saving valuable development time and reducing resource consumption.

The offending query offers nothing to this specific context since hasTable returns a boolean. Other functions that utilise the table size will continue to operate as usual.

Looking forward to hear your thoughts.

[apspan]

The added latency is nearly 640% up. It's not trivial.

[GrahamCampbell]

Ok, thanks for the extra details. I don't feel strongly here, either way.