yajra / laravel-oci8

Oracle DB driver for Laravel via OCI8
https://yajrabox.com/docs/laravel-oci8
MIT License
829 stars 237 forks source link

Improving pagination performance #870

Open alexc-jeromes opened 1 month ago

alexc-jeromes commented 1 month ago

Pagination is insanely slow.

This code:

$items->paginate(25)

and produces:

select t2.* from ( select rownum AS "rn", t1.* from (select * from table where "some_field" is not null and "other_field" is null) t1 ) t2 where t2."rn" between 1 and 25

then takes 23 seconds in PHP or Oracle in our DB of 500k records.

Change to ROW_NUMBER() OVER

select t2.* from ( select ROW_NUMBER() OVER (ORDER BY t1.primary_key_field) AS "rn", t1.* from (select * from table where "some_field" is not null and "other_field" is null) t1 ) t2 where t2."rn" between 1 and 25

and it takes 0.8s.

GPT4 suggests:

protected function compileTableExpression($sql, $constraint, $query)
{
    // Case for fetching a single row
    if ($query->limit == 1 && is_null($query->offset)) {
        return "select * from ({$sql}) where rownum {$constraint}";
    }

    // Case for pagination with limit and offset
    if (!is_null($query->limit) && !is_null($query->offset)) {
        $start  = $query->offset + 1;
        $finish = $query->offset + $query->limit;

        // Using ROW_NUMBER() and CTE for efficient pagination
        return "
            WITH query_cte AS (
                SELECT t1.*, ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) AS rn
                FROM ({$sql}) t1
            )
            SELECT *
            FROM query_cte
            WHERE rn BETWEEN {$start} AND {$finish}
        ";
    }

    // Fallback for other cases
    return "
        WITH query_cte AS (
            SELECT t1.*, ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) AS rn
            FROM ({$sql}) t1
        )
        SELECT *
        FROM query_cte
        WHERE rn {$constraint}
    ";
}
alexc-jeromes commented 1 month ago

This is working in production fairly well.

    protected function compileTableExpression($sql, $constraint, $query)
    {
        if ($query->limit == 1 && is_null($query->offset)) {
            return "select * from ({$sql}) where rownum {$constraint}";
        }

        // Apply ROW_NUMBER() for pagination
        $orderBy = $this->compileOrders($query, $query->orders);

        // If no ORDER BY is specified, use ROWID for ordering
        if (empty($orderBy)) {
            $orderBy = "order by ROWID";
        }

        return "select * from (
                    select t1.*, row_number() over ({$orderBy}) as row_num
                    from ({$sql}) t1
                ) where row_num {$constraint}";
    }

Example query generated, down from 20s to 650ms:

select * from (
    select t1.*, row_number() over (order by ROWID) as row_num
    from (select * from "INV"."ITM" where "ITM_CD" is not null and "DROP_DT" is null) t1
) where row_num between 126 and 150

Implemented in AppServiceProvider like so:

class_alias (App\Database\Query\Grammars\OracleGrammar::class, \Yajra\Oci8\Query\Grammars\OracleGrammar::class);
yajra commented 1 month ago

@alexc-jeromes thank you for taking your time in reviewing this issue. Can you please submit a PR, I will review it asap.

alexc-jeromes commented 1 month ago

Welcome! We literally just override this function.

It would be worth testing this out though before a PR, as although it works for us, it may encounter other scenarios where it doesn't (i don't know the full scope of the package).

PR submitted: https://github.com/yajra/laravel-oci8/pull/878

mixaster commented 1 month ago

Just FYI, I tried implementing @alexc-jeromes override in our app, and it was great for a slow query issue we were having, but unfortunately we got errors in other queries, specifically ones with subqueries. For instance, we got ORA-01445 in a few places.

I'm doing a lot of research, and I'm wondering if modifying compileRowConstraint() might be better, using OFFSET and FETCH? I'm not an Oracle expert, by any means...

alexc-jeromes commented 1 month ago

@mixaster Yeh i suspected that might be the case. We're using it with quite simple queries and the solution is only needed for record browsing without complexity, so it will need a bit of tweaking. I can see the PR failed too. I'm not heavy enough into OCI8 to know the best way to do Oracle-specific driver work. We're moving to PG thankfully.

github-actions[bot] commented 1 week ago

This issue is stale because it has been open for 30 days with no activity.