Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize Row Limiting Clause in Oracle Database 12c #2649

Closed
stchr opened this issue Feb 8, 2017 · 12 comments
Closed

Optimize Row Limiting Clause in Oracle Database 12c #2649

stchr opened this issue Feb 8, 2017 · 12 comments

Comments

@stchr
Copy link

stchr commented Feb 8, 2017

In 12c Release 1 (12.1.0.1) Oracle introduced row_limiting_clause, which is faster and easier to understand.

Doctrine\DBAL\Platforms\OraclePlatform::doModifyLimitQuery() should be overwritten with new Doctrine\DBAL\Platforms\Oracle12c1Platform::doModifyLimitQuery()

stchr pushed a commit to stchr/dbal that referenced this issue Feb 8, 2017
@Ocramius
Copy link
Member

Ocramius commented Feb 8, 2017

faster and easier to understand.

Unless there is a functional need for a change, I suggest letting doModifyLimitQuery untouched even for newer OracleDB versions: that piece of logic is pandora's box, and it's good enough if it works reliably.

@stchr
Copy link
Author

stchr commented Feb 8, 2017

protected function doModifyLimitQuery($query, $limit, $offset = null)
    {
        if ($limit === null) {
            return $query;
        }
        $limit = (int)$limit;
        $offset = (int)$offset;

        if (preg_match('/^\s*SELECT/i', $query)) {
            if (!preg_match('/\sFROM\s/i', $query)) {
                $query .= ' FROM dual';
            }
            if ($limit > 0) {
                $query .= ' OFFSET ' . $offset . ' ROWS FETCH NEXT ' . $limit . ' ROWS ONLY';
            }
        }

        return $query;
    }

vs.

protected function doModifyLimitQuery($query, $limit, $offset = null)
    {
        $limit = (int) $limit;
        $offset = (int) $offset;

        if (preg_match('/^\s*SELECT/i', $query)) {
            if (!preg_match('/\sFROM\s/i', $query)) {
                $query .= " FROM dual";
            }
            if ($limit > 0) {
                $max = $offset + $limit;
                $column = '*';
                if ($offset > 0) {
                    $min = $offset + 1;
                    $query = 'SELECT * FROM (SELECT a.' . $column . ', rownum AS doctrine_rownum FROM (' .
                            $query .
                            ') a WHERE rownum <= ' . $max . ') WHERE doctrine_rownum >= ' . $min;
                } else {
                    $query = 'SELECT a.' . $column . ' FROM (' . $query . ') a WHERE ROWNUM <= ' . $max;
                }
            }
        }

        return $query;
    }

@Ocramius
Copy link
Member

Ocramius commented Feb 8, 2017

That still means ending up with two implementations that need to be maintained: can be considered when the supported version of OracleDB is ALWAYS > 12c Release 1 (12.1.0.1)

@stchr
Copy link
Author

stchr commented Feb 8, 2017

Ok, thanks.

@rjd22
Copy link

rjd22 commented Mar 11, 2022

@Ocramius could we maybe revisit this? I ran into trouble with combining row locking with ROWNUM and using ROWS FETCH might maybe solve this issue.

I will need to do some testing but would supporting more modern Oracle syntax in maybe a separate driver be open for discussion since I'm running into a lot of issues with compatibility with the old syntax.

Example of issue with locking in symfony/messenger

symfony/symfony#45714

@Ocramius
Copy link
Member

@rjd22 send a patch: I need to be paid looooooooots of money to move even a finger, when something mentions "Oracle" in it.

@rjd22
Copy link

rjd22 commented Mar 11, 2022

@Ocramius I hear you. I feel lots of pain working with oracle daily. We want to switch but these are now the cards I've been dealt :(.

I will make a patch. Would making a separate driver be recommended to keep backwards compatibility, like done with MySQL?

@Ocramius
Copy link
Member

Possibly: beware that I'm no longer the maintainer here, so I endorse creating a clear issue that goes to the point, before throwing more work at it.

@stchr
Copy link
Author

stchr commented Mar 11, 2022

Would be glad to help if needed 👍

@rjd22
Copy link

rjd22 commented Mar 11, 2022

After checking it seems the fix of it is already in the next major version of dbal. So with dbal 4.* This will be solved.

@stchr
Copy link
Author

stchr commented Mar 19, 2022

Just for reference: This was fixed with #5150

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2022
@morozov morozov closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants