staudenmeir / laravel-adjacency-list

Recursive Laravel Eloquent relationships with CTEs
MIT License
1.35k stars 110 forks source link

Can't add constraints to initial query and recursive query of a graph relation #221

Closed alexdemers closed 9 months ago

alexdemers commented 9 months ago

I discovered this library for hopefully switch all of our tree queries to use MySQL's with expressions using this library. For our use, we have a tree of files and folders for every user of our application.

The code works, but it is not optimized for performance. The added constraint for the user id is added at the end. I want to add it to the initial query and the recursive query.

Basic structure

user_fileables_tree

`fileable_id`: int
`fileable_type`: string ("folder" or "file")
`parent_id`: int
`user_id`: int

Models/Folder.php

public Folder {
    use HasGraphRelationships;

    public function parents() {
        return $this->ancestors();
    }
}

main.php

$folder = Folder::find(1234);
// we need to scope for the user's tree
$parents = $folders->parents()->where('pivot_user_id', $user->getKey());

Executed query

(see comments in the query)

WITH RECURSIVE
  `laravel_cte` AS (
    (
      SELECT
        `folders`.*,
        -1 AS `depth`,
        CAST(`folders`.`ID` AS CHAR(65535)) AS `path`,
        `user_fileables_tree`.`parent_id` AS `pivot_parent_id`,
        `user_fileables_tree`.`fileable_id` AS `pivot_fileable_id`,
        `user_fileables_tree`.`user_id` AS `pivot_user_id`,
        `user_fileables_tree`.`fileable_type` AS `pivot_fileable_type`
      FROM
        `folders`
        INNER JOIN `user_fileables_tree` ON `folders`.`id` = `user_fileables_tree`.`parent_id`
      WHERE
        # I want the constraint to be here
        # `user_fileables_tree`.`user_id` = 1
        `user_fileables_tree`.`fileable_id` = 1234
    )
    UNION ALL
    (
      SELECT
        `folders`.*,
        `depth` - 1 AS `depth`,
        concat (`path`, /, `folders`.`id`),
        `user_fileables_tree`.`parent_id` AS `pivot_parent_id`,
        `user_fileables_tree`.`fileable_id` AS `pivot_fileable_id`,
        `user_fileables_tree`.`user_id` AS `pivot_user_id`,
        `user_fileables_tree`.`fileable_type` AS `pivot_fileable_type`
      FROM
        `folders`
        INNER JOIN `user_fileables_tree` ON `folders`.`id` = `user_fileables_tree`.`parent_id`
        INNER JOIN `laravel_cte` ON `laravel_cte`.`id` = `user_fileables_tree`.`fileable_id`
      WHERE
        # I want the constraint to be here
        # `user_fileables_tree`.`user_id` = 1
    )
  )
SELECT
  *
FROM
  `laravel_cte`
WHERE
  `pivot_user_id` = 1 # I don't want this to be here, it is too slow

I tried adding subgraph(fn($query) => $query->where("pivot_user_id", $user->getKey())) to my ancestors() relationship but it adds another query to the with expression as shown below, which errors out because there are 2 tables named laravel_cte:

WITH RECURSIVE `laravel_cte` AS ((
        SELECT
            `folders`.*,
            - 1 AS `depth`,
            cast(`folders`.`id` AS char(65535)) AS `path`,
            `user_fileables_tree`.`parent_id` AS `pivot_parent_id`,
            `user_fileables_tree`.`fileable_id` AS `pivot_fileable_id`,
            `user_fileables_tree`.`user_id` AS `pivot_user_id`,
            `user_fileables_tree`.`fileable_type` AS `pivot_fileable_type`
        FROM
            `folders`
            INNER JOIN `user_fileables_tree` ON `folders`.`id` = `user_fileables_tree`.`parent_id`
        WHERE
            `user_fileables_tree`.`fileable_id` = 1234)
    UNION ALL (
        SELECT
            `folders`.*,
            `depth` - 1 AS `depth`,
            concat(`path`, '/', `folders`.`id`),
            `user_fileables_tree`.`parent_id` AS `pivot_parent_id`,
            `user_fileables_tree`.`fileable_id` AS `pivot_fileable_id`,
            `user_fileables_tree`.`user_id` AS `pivot_user_id`,
            `user_fileables_tree`.`fileable_type` AS `pivot_fileable_type`
        FROM
            `folders`
            INNER JOIN `user_fileables_tree` ON `folders`.`id` = `user_fileables_tree`.`parent_id`
            INNER JOIN `laravel_cte` ON `laravel_cte`.`id` = `user_fileables_tree`.`fileable_id`)
),
`laravel_cte` AS ((
        SELECT
            `laravel_cte`.*,
            0 AS `depth`,
            cast(`laravel_cte`.`id` AS char(65535)) AS `path`,
            cast(NULL AS signed) AS `pivot_parent_id`,
            cast(NULL AS signed) AS `pivot_fileable_id`,
            cast(NULL AS signed) AS `pivot_user_id`,
            cast(NULL AS char(65535)) AS `pivot_fileable_type`
        FROM
            `laravel_cte`
        WHERE
                        # it adds it here
            `pivot_user_id` = 1)
    UNION ALL (
        SELECT
            `laravel_cte`.*,
            `depth` + 1 AS `depth`,
            concat(`path`, '/', `laravel_cte`.`id`),
            `user_fileables_tree`.`parent_id` AS `pivot_parent_id`,
            `user_fileables_tree`.`fileable_id` AS `pivot_fileable_id`,
            `user_fileables_tree`.`user_id` AS `pivot_user_id`,
            `user_fileables_tree`.`fileable_type` AS `pivot_fileable_type`
        FROM
            `laravel_cte`
            INNER JOIN `user_fileables_tree` ON `laravel_cte`.`id` = `user_fileables_tree`.`fileable_id`
            INNER JOIN `laravel_cte` ON `laravel_cte`.`id` = `user_fileables_tree`.`user_id`)
)
SELECT
    *
FROM
    `laravel_cte`
WHERE
  # also adds it here
  `pivot_user_id` = 1

I know there's a static::$recursiveQueryConstraint that you can assign, but it's not per-query based, it's a global constraint.

So basically my question is: how do I add constraints to the initial query and the recursive query without adding them to the "end" query to optimize performance? I wish a whereInitial($constraint) and whereRecursive($constraint) would exist. Maybe I'm missing out on something obvious but I've read through the docs and code...

alexdemers commented 9 months ago

To add performance benchmarks, considering my user_fileables_tree has ~120k rows on my development machine (production is millions of rows), adding constraints to the end query takes ~1.2 seconds on average and having them in the initial query and recursive query takes about 0.02s (20ms).

staudenmeir commented 9 months ago

Hi @alexdemers,

I know there's a static::$recursiveQueryConstraint that you can assign, but it's not per-query based, it's a global constraint.

The constraint can be global, but with it's only applied to a single query typically: https://github.com/staudenmeir/laravel-adjacency-list?tab=readme-ov-file#graphs-recursive-query-constraints

Nevertheless, it's actually not possible to add the constraint to the initial query in your particular case. I've already been working on the missing feature and I'll prioritize it.

Thanks for sharing the benchmark.

staudenmeir commented 9 months ago

I've released a new version with withQueryConstraint() that allows you to add a constraint to both the initial and the recursive query: https://github.com/staudenmeir/laravel-adjacency-list?tab=readme-ov-file#graphs-initial--recursive-query-constraints

$parents = Folder::withQueryConstraint(function (Builder $query) use ($user) {
   $query->where('user_fileables_tree.user_id', $user->getKey());
}, function () use ($folder) {
   return $folder->parents;
});
alexdemers commented 9 months ago

Thank you very much, I'll try this out! This library really helps me query my hierarchy without having to rewrite the table structure! Great work!

alexdemers commented 7 months ago

This works for per-query basis, but how can I integrate this for relationships? What would be nice is:

$folders->descendants()
  ->withQueryConstraint(fn($query) => $query->where('user_fileables_tree.user_id', $user->getKey());
staudenmeir commented 7 months ago

Due to the way Laravel relationships work, that's not possible, unfortunately.