Filament•2y ago

Duplicate queries despite closures and static variables

I changed to closures, which reduced duplicate queries. Then I added some local caching for the current request by using static variables, further reducing duplicate queries. Still, the query inside options() is called 3 times by hidden(). Is there any way to get rid of these duplicate queries?

Select::make('arrangement_id')
    ->options(function (Closure $get) {
        static $result = null;
        $result ??= Arrangement
            ::forArrangementTypeId($get('arrangement_type_id'))
            ->pluck('arrangement', 'arrangement_id');
        return $result;
    })
    ->required()
    ->hidden(function (Select $component) {
        static $result = null;
        $result ??= count($component->getOptions());
        return $result == 0;
    })

Select::make('arrangement_id')
    ->options(function (Closure $get) {
        static $result = null;
        $result ??= Arrangement
            ::forArrangementTypeId($get('arrangement_type_id'))
            ->pluck('arrangement', 'arrangement_id');
        return $result;
    })
    ->required()
    ->hidden(function (Select $component) {
        static $result = null;
        $result ??= count($component->getOptions());
        return $result == 0;
    })

18 Replies

MrHex•2y ago

One way to further reduce duplicate queries would be to move the query inside the options() closure to a higher level where it can be cached and reused across requests. For example, you could create a separate function outside of the component definition that retrieves the options for a given arrangement_type_id and caches the result. Then, you can call this function inside the options() closure to get the cached options instead of querying the database. Here's an example implementation:

function getArrangementOptions($arrangementTypeId)
{
    static $cache = [];

    if (!isset($cache[$arrangementTypeId])) {
        $cache[$arrangementTypeId] = Arrangement
            ::forArrangementTypeId($arrangementTypeId)
            ->pluck('arrangement', 'arrangement_id')
            ->toArray();
    }

    return $cache[$arrangementTypeId];
}

Select::make('arrangement_id')
    ->options(function (Closure $get) {
        $arrangementTypeId = $get('arrangement_type_id');
        return getArrangementOptions($arrangementTypeId);
    })
    ->required()
    ->hidden(function (Select $component) {
        return count($component->getOptions()) == 0;
    });

function getArrangementOptions($arrangementTypeId)
{
    static $cache = [];

    if (!isset($cache[$arrangementTypeId])) {
        $cache[$arrangementTypeId] = Arrangement
            ::forArrangementTypeId($arrangementTypeId)
            ->pluck('arrangement', 'arrangement_id')
            ->toArray();
    }

    return $cache[$arrangementTypeId];
}

Select::make('arrangement_id')
    ->options(function (Closure $get) {
        $arrangementTypeId = $get('arrangement_type_id');
        return getArrangementOptions($arrangementTypeId);
    })
    ->required()
    ->hidden(function (Select $component) {
        return count($component->getOptions()) == 0;
    });

In this implementation, the getArrangementOptions() function uses a static cache variable to store the retrieved options for each arrangement_type_id. If the options for a given arrangement_type_id haven't been retrieved yet, the function queries the database and caches the result. Inside the options() closure, we simply call getArrangementOptions() with the arrangement_type_id parameter to get the cached options. The hidden() closure doesn't need to change since it already uses the getOptions() method to check if there are any options available. By caching the options at a higher level and reusing them across requests, you can further reduce duplicate queries and improve performance.

JohnOP•2y ago

That's a possibility indeed. Maybe I'll even implement a generic model cache, or even a package. Thanks! Any possibilities from within the Filament framework?

Dan Harrin•2y ago

we dont cache within filament in case you want to implement dynamic options

JohnOP•2y ago

Ok, I'll see if I can implement some cache-per-request layer myself. What about relationships? As soon as I introduce a

Repeater::make('records')->relationship('records')

Repeater::make('records')->relationship('records')

I get 5 duplicate queries. I wouldn't know how to get rid of those. I came up with this simple solution to prevent duplicate queries in most cases:

Select::make('arrangement_id')
    ->options(fn(Closure $get) => $GLOBALS[__FILE__ . __LINE__] ??=
        Arrangement
            ::forArrangementTypeId($get('arrangement_type_id'))
            ->pluck('arrangement', 'arrangement_id')
    );

Select::make('arrangement_id')
    ->options(fn(Closure $get) => $GLOBALS[__FILE__ . __LINE__] ??=
        Arrangement
            ::forArrangementTypeId($get('arrangement_type_id'))
            ->pluck('arrangement', 'arrangement_id')
    );

The first time during the request it will store the result in a global variable. Every subsequent call will use the stored result. The __FILE__ . __LINE__ part is just to easily copy paste and have a unique key. It removes the duplicate queries that remained while using a static variable inside the closure. The only duplicates left now are the ones from ->relation(). Any ideas to remove those are welcome.

Dan Harrin•2y ago

maybe the spatie once() helper

JohnOP•2y ago

I did't know that one. It seems to me it's doing something similar to what I've done. At least the result is the same. I don't see how I would implement once() in case of a Repeater::make('records')->relationship() ?

Dan Harrin•2y ago

5 duplicate queries when in the lifecycle?

JohnOP•2y ago

4x this:

select `arrangement`, `arrangement_id` from `ctlv__arrangement` where `arrangement_type_id` = 2 and exists (select * from `ctlv__decision` where `ctlv__arrangement`.`arrangement_id` = `ctlv__decision`.`arrangement_id` and `active` = 1)
620μs
/app/Filament/Resources/RequestResource/Steps/ArrangementStep.php:70
tommy_breda
Metadata
Bindings     
0. 2
1. 1
Backtrace     
13. /app/Filament/Resources/RequestResource/Steps/ArrangementStep.php:70
14. /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:36
15. /vendor/laravel/framework/src/Illuminate/Container/Util.php:41
16. /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:81
17. /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:35

select `arrangement`, `arrangement_id` from `ctlv__arrangement` where `arrangement_type_id` = 2 and exists (select * from `ctlv__decision` where `ctlv__arrangement`.`arrangement_id` = `ctlv__decision`.`arrangement_id` and `active` = 1)
620μs
/app/Filament/Resources/RequestResource/Steps/ArrangementStep.php:70
tommy_breda
Metadata
Bindings     
0. 2
1. 1
Backtrace     
13. /app/Filament/Resources/RequestResource/Steps/ArrangementStep.php:70
14. /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:36
15. /vendor/laravel/framework/src/Illuminate/Container/Util.php:41
16. /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:81
17. /vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:35

without the GLOBALS check. While rendering the EditRequest page. Sorry, you asked about the repeater... 4x this on the ->relation() while rendering the EditRequest page:

select * from `sys__file` inner join `ctlv__request_document` on `sys__file`.`file_id` = `ctlv__request_document`.`file_id` where `ctlv__request_document`.`request_id` = 269
600μs
/vendor/filament/forms/src/Components/Repeater.php:476
Backtrace     
14. /vendor/filament/forms/src/Components/Repeater.php:476
15. /vendor/filament/forms/src/Components/Repeater.php:245
16. /vendor/filament/forms/src/Concerns/CanBeValidated.php:39
17. /vendor/filament/forms/src/Concerns/CanBeValidated.php:44
18. /vendor/filament/forms/src/Concerns/CanBeValidated.php:44

select * from `sys__file` inner join `ctlv__request_document` on `sys__file`.`file_id` = `ctlv__request_document`.`file_id` where `ctlv__request_document`.`request_id` = 269
600μs
/vendor/filament/forms/src/Components/Repeater.php:476
Backtrace     
14. /vendor/filament/forms/src/Components/Repeater.php:476
15. /vendor/filament/forms/src/Components/Repeater.php:245
16. /vendor/filament/forms/src/Concerns/CanBeValidated.php:39
17. /vendor/filament/forms/src/Concerns/CanBeValidated.php:44
18. /vendor/filament/forms/src/Concerns/CanBeValidated.php:44

Dan Harrin•2y ago

are you on the latest filament version?

JohnOP•2y ago

I am now. Same story.

Dan Harrin•2y ago

im not sure then, it sounds like a bug but there might be a good reason for it.

JohnOP•2y ago

Should I add an issue for it on Github?

Dan Harrin•2y ago

if you want, its not a priority to fix though as it doesnt actually stop you from doing anything you can submit a PR to fix if you want, that would be better

JohnOP•2y ago

Ok. I'll use my GLOBAL hack to prevent the majority of duplicate queries. If I still get in trouble somewhere down the road I'll look into it. Thanks!

Dan Harrin•2y ago

if i were you, id prioritise code quality over microoptimizations and not bother to be honest 😁

JohnOP•2y ago

I guess you're right. It's my first Filament project of ~~hopefully~~ probably many. So I want to set up everything really tight in order to prevent troubles way down the road.

Dan Harrin•2y ago

yeah but i wouldnt worry about a probably tiny query running a handful of times just my opinion with priorities

JohnOP•2y ago

I agree a 1+n is way worse

Gaming

Programming

Duplicate queries despite closures and static variables