yajra / laravel-oci8

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

Lock for update implementation on 9x #876

Open gdonnari opened 1 month ago

gdonnari commented 1 month ago

Summary of problem or feature request

I´m facing this issue with yajra/laravel-oci8 9x I can see it was fixed here The fix was merged in 10x, but not in 9x. It is possible to apply this fix on latest 9x branch?

System details

yajra commented 1 month ago

Sure, will check if the PR can be merged to 9.x when I get the chance. If you can, please do not hesitate to submit a PR. Thanks!

gdonnari commented 1 month ago

Hi Arjay,

When testing the merge locally I detected that this fix brokens other use cases.

If the query uses a distint, orderby, groupby, etc then Oracle raises ORA-02014 when for update is also set.

For example, this code extracted from Laravel DatabaseQueue.php:

protected function getNextAvailableJob($queue)
    {
        $job = $this->database->table($this->table)
                    ->lock($this->getLockForPopping())
                    ->where('queue', $this->getQueue($queue))
                    ->where(function ($query) {
                        $this->isAvailable($query);
                        $this->isReservedButExpired($query);
                    })
                    ->orderBy('id', 'asc')
                    ->first();
        ...
    }

Currently the generated SQL is:

select * from "TBL_JOB" 
where "QUEUE" = :p0 and (("RESERVED_AT" is null and "AVAILABLE_AT" <= :p1) or ("RESERVED_AT" <= :p2)) 
order by "ID" asc for update

This query does not honor ->first() which is the original issue reported, and locks all the returned records instead of the first one. This causes large processes to take a huge ammount of time to complete.

Now, with the fix applied it honors ->first() but it raises ORA-02014. The generated query is:

select * from (
select * from "TBL_JOB" 
where "QUEUE" = :p0 and (("RESERVED_AT" is null and "AVAILABLE_AT" <= :p1) or ("RESERVED_AT" <= :p2)) 
order by "ID" asc
) where rownum = 1 
for update;

So, I'm not sure what the correct patch is, in order to fully honor the query builder in the example. Think this require futher analisys.

yajra commented 1 month ago

I think this PR https://github.com/yajra/laravel-oci8/pull/834 also needs to be merged.