Bug: Query Builder limit() problem with OCI8

[rutgers-master]

PHP Version

8.3

CodeIgniter4 Version

4.6.0

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

Oracle

What happened?

When using Query Builder (OCI8) on an Oracle database, it seems like the limit() function is not working properly when returning results for anything after the first page. In other words, if I do a limit(2,0), I'll get back the first 2 results correctly, however if I do a limit(2,2), instead of getting back the next page of 2 results, instead I get back 3 results and one of the results is a duplicate from the previous page of results.

Steps to Reproduce

Using a Model that is for an Oracle table, simply do:

$results = $builder->select("field")->limit(2,0)->get()->getResult();
// Print the results
print_r($results);
// Then do it again to get the next 2 results:
$results = $builder->select("field")->limit(2,2)->get()->getResult();
// Print the results
print_r($results);
// You'll get back 3 results instead of 2 results.

Expected Output

The expected output would be to get back just 2 results when doing limit(2,2), and this can easily be fixed by modifying the Codeigniter4/system/Database/OCI8/Builder.php file and changing this line:
$limitTemplateQuery = 'SELECT * FROM (SELECT INNER_QUERY.*, ROWNUM RNUM FROM (%s) INNER_QUERY WHERE ROWNUM < %d)' . ($offset !== 0 ? ' WHERE RNUM >= %d' : '');
To this line:
$limitTemplateQuery = 'SELECT * FROM (SELECT INNER_QUERY.*, ROWNUM RNUM FROM (%s) INNER_QUERY WHERE ROWNUM < %d)' . ($offset !== 0 ? ' WHERE RNUM > %d' : '');

Note: All I did was change "RNUM >= %d" to "RNUM > %d".

Anything else?

If you need more details, please let me know. Unfortunately I don't know the specific version of the Oracle server I am accessing, I don't manage the Oracle server itself.

[michalsn]

A version would be useful, as our tests already cover this: https://github.com/codeigniter4/CodeIgniter4/blob/develop/tests/system/Database/Live/LimitTest.php#L25

[michalsn]

Try to run:

SELECT BANNER FROM V$VERSION;

[rutgers-master]

Thank you for that SQL statement:

Oracle Database 19c Enterprise Edition Release 19.0.0.0.0 - Production

[michalsn]

That is strange. Can you check what you get, when you call:

// assuming you use the default connection
$db = db_connect();
echo $db->getVersion();
// and
echo oci_server_version($db->connID);

In general, the $limitTemplateQuery should not be used to determine the LIMIT part in your query. Since your version is greater than 12.1.

https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/Database/OCI8/Builder.php#L217-L224

Can you change this method and check if it will work correctly?

protected function _limit(string $sql, bool $offsetIgnore = false): string
{
    $offset = (int) ($offsetIgnore === false ? $this->QBOffset : 0);

    // OFFSET-FETCH can be used only with the ORDER BY clause
    if (empty($this->QBOrderBy)) {
        $sql .= ' ORDER BY 1';
    }

    return $sql . ' OFFSET ' . $offset . ' ROWS FETCH NEXT ' . $this->QBLimit . ' ROWS ONLY';
}

Since we no longer support versions below 12.1 it would make everything simpler.

[rutgers-master]

Interesting results:

$db = \Config\Database::connect([ "DSN" => "//my.server.here:1521/mydb", "hostname" => "my.server.here", "username" => "my_username", "password" => "my_password", "database" => "mydb", "DBDriver" => "OCI8", "DBPrefix" => "", "pConnect" => true, "DBDebug" => true, "cacheOn" => false, "cacheDir" => "", "charset" => "utf8", "DBCollat" => "utf8_general_ci", "swapPre" => "", "encrypt" => false, "compress" => false, "strictOn" => false, "failover" => [], "port" => 1521 ]);
echo "DB Version = ".$db->getVersion();
echo "DB Version = ".oci_server_version($db->connID);

Results are:

DB Version = 
With an error: TypeError: oci_server_version(): Argument #1 ($connection) must be of type resource, null given

Looking in that getVersion() method, $this->dataCache is an empty array for me, and it's getting tripped up on the $this->connID check because it is null. If I look at the contents of $db, it contains:

{"connID":false,"resultID":false,"protectIdentifiers":true,"escapeChar":"\"","likeEscapeStr":" ESCAPE '%s' ","likeEscapeChar":"!","dataCache":[],"transEnabled":true,"transStrict":true,"commitMode":32,"lastInsertedTableName":null}

Yet I'm able to immediately do a $builder = $db->table("mytable"); $results = $builder->select("field")->get()-getResult(); and get back results from the database. So even though it says there's no connID...the connection is still there.

And yes, if I change the _limit method to your new version that you provided, that does work correctly and provides the correct results.

Thank you for your help with this and if there's any more testing I can do for you, please let me know.

[michalsn]

Oh, ok I see it now. The connection is established only right before we execute the query, so obviously there is no connID when we try to get the version. The same problem occurred in the code I posted to debug the issue.

I will prepare a fix later.

Thank you for the report and feedback.