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

Fix truncated oracle fetch result #4581

Merged
merged 1 commit into from Apr 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/Driver/OCI8/Exception/PHPError.php
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Driver\OCI8\Exception;

use Doctrine\DBAL\Driver\AbstractException;

use function sprintf;

/**
* This exception is used for any PHP error that the
* OCI8 extension doesn't report via oci_error()
*
* @internal
*
* @psalm-immutable
*/
final class PHPError extends AbstractException
{
public static function new(
string $preambleMessage,
int $errno,
string $errstr,
string $errfile,
int $errline
): self {
return new self(sprintf(
'%s Error number: %s, Error String: %s, Error File: %s, Error Line: %s',
$preambleMessage,
$errno,
$errstr,
$errfile,
$errline
));
}
}
53 changes: 50 additions & 3 deletions src/Driver/OCI8/Result.php
Expand Up @@ -5,14 +5,19 @@
namespace Doctrine\DBAL\Driver\OCI8;

use Doctrine\DBAL\Driver\FetchUtils;
use Doctrine\DBAL\Driver\OCI8\Exception\PHPError;
use Doctrine\DBAL\Driver\Result as ResultInterface;

use function oci_cancel;
use function oci_fetch_all;
use function oci_fetch_array;
use function oci_num_fields;
use function oci_num_rows;
use function preg_match_all;
use function restore_error_handler;
use function set_error_handler;

use const E_WARNING;
use const OCI_ASSOC;
use const OCI_FETCHSTATEMENT_BY_COLUMN;
use const OCI_FETCHSTATEMENT_BY_ROW;
Expand Down Expand Up @@ -115,10 +120,52 @@ public function free(): void
*/
private function fetch(int $mode)
{
return oci_fetch_array(
$this->statement,
$mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS
set_error_handler(
static function (int $errno, string $errstr, string $errfile, int $errline): bool {
/*
* Screen PHP error messages for any error that the OCI8 extension
* doesn't report via oci_error()
*
* @link http://www.dba-oracle.com/t_error_code_list.htm
*/
if (preg_match_all('/ORA-(\d+)/', $errstr, $matches) === 0) {
// If no ORA error codes are present in error message,
// skip this error and bubble this error up to
// the normal error handler
return false;
}

foreach ($matches[1] as $oraErrorCode) {
switch ($oraErrorCode) {
case '04061':
case '04065':
case '04068':
throw PHPError::new(
'There was an error before all rows could be fetched.',
$errno,
$errstr,
$errfile,
$errline
);
}
}

// If ORA error codes in message do not match specified blocklist
// ORA error codes, skip this error and bubble this error up to
// the normal error handler
return false;
},
E_WARNING
);

try {
return oci_fetch_array(
$this->statement,
$mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS
);
} finally {
restore_error_handler();
}
}

/**
Expand Down
174 changes: 174 additions & 0 deletions tests/Functional/Driver/OCI8/ResultTest.php
@@ -0,0 +1,174 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Functional\Driver\OCI8;

use Doctrine\DBAL\Driver\OCI8\Driver;
use Doctrine\DBAL\Exception\DriverException;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use Doctrine\DBAL\Tests\TestUtil;
use Generator;

use function ini_get;
use function sprintf;

/**
* @requires extension oci8
*/
class ResultTest extends FunctionalTestCase
{
/**
* Database connection parameters for functional test case
*
* @var array<string,mixed>
*/
private $connectionParams;

protected function setUp(): void
{
parent::setUp();
$this->connectionParams = TestUtil::getConnectionParams();

if ($this->connection->getDriver() instanceof Driver) {
return;
}

self::markTestSkipped('oci8 only test.');
morozov marked this conversation as resolved.
Show resolved Hide resolved
}

protected function tearDown(): void
{
$this->connection->executeQuery(sprintf(
'DROP FUNCTION test_oracle_fetch_failure',
$this->connectionParams['user']
));
$this->connection->executeQuery(sprintf(
'DROP TYPE return_numbers',
$this->connectionParams['user']
));

parent::tearDown();
}

/**
* This test will recreate the case where a data set that is larger than the
* oci8 default prefetch is invalidated on the database after a fetch has begun,
* but before the fetch has completed.
*
* Note that this test requires 2 separate user connections so that the
* pipelined function can be changed mid fetch.
*
* @dataProvider dataProviderForTestTruncatedFetch
*/
public function testTruncatedFetch(
bool $invalidateDataMidFetch
): void {
if ($invalidateDataMidFetch) {
$this->expectException(DriverException::class);
$this->expectErrorMessageMatches(
'/^An exception occurred in the driver: '
. 'There was an error before all rows could be fetched. '
. 'Error number: 2, Error String: oci_fetch_array.*/'
);
}

// Create a pipelined funtion that returns 10 rows more than the
// oci8 default prefetch
$this->createReturnTypeNeededForPipelinedFunction();
$expectedTotalRowCount = (int) ini_get('oci8.default_prefetch') + 10;
$this->createOrReplacePipelinedFunction($expectedTotalRowCount);

// Create a separate connection from that used to create/update the function
// This must be a different user with permissions to change the given function
$separateConnection = TestUtil::getPrivilegedConnection();

// Query the pipelined function to get initial dataset
$statement = $separateConnection->prepare(sprintf(
'SELECT * FROM TABLE(%s.test_oracle_fetch_failure())',
$this->connectionParams['user']
));
$result = $statement->execute();

// Access the first result to cause the first X rows to be prefetched
// as defined by oci8.default_prefetch (often 100 rows)
$firstNumber = $result->fetchOne();

if ($invalidateDataMidFetch) {
// Invalidate the original dataset by changing the pipelined function
// after the initial prefetch that caches locally the first X results
$this->createOrReplacePipelinedFunction($expectedTotalRowCount + 10);
}

while ($number = $result->fetchOne()) {
// Attempt to access all remaining rows from the original fetch
// The rows locally cached from the default prefetch will first be used
// but when the result attempts to get the remaining 10 rows beyond
// the first prefetch, nothing will be returned
//
// PHP oci8 oci_fetch_array will issue a PHP E_WARNING when the 2nd prefetch occurs
// oci_fetch_array(): ORA-04068: existing state of packages has been discarded
// ORA-04061: existing state of function "ROOT.TEST_ORACLE_FETCH_FAILURE" has been invalidated
// ORA-04065: not executed, altered or dropped function "ROOT.TEST_ORACLE_FETCH_FAILURE"
//
// If there was no issue, this should have returned rows totalling 10
// higher than the oci8 default prefetch
continue;
}

$this->assertEquals(
$expectedTotalRowCount,
$result->rowCount(),
sprintf(
'Expected to have %s total rows fetched but only found %s rows fetched',
$expectedTotalRowCount,
$result->rowCount()
)
);
}

public function dataProviderForTestTruncatedFetch(): Generator
{
yield 'it should return all rows if no data invalidation occurs'
=> [false];

yield 'it should convert oci8 data invalidation error to DriverException'
=> [true];
}

private function createReturnTypeNeededForPipelinedFunction(): void
{
$this->connection->executeQuery(
'CREATE TYPE return_numbers AS TABLE OF NUMBER(11)'
);
}

/**
* This will create a pipelined function that returns X rows with
* each row returning a single column_value of that row's row number.
* The total number of rows returned is equal to $totalRowCount.
*/
private function createOrReplacePipelinedFunction(int $totalRowCount): void
{
$this->connection->executeQuery(sprintf(
'CREATE OR REPLACE FUNCTION test_oracle_fetch_failure
RETURN return_numbers PIPELINED
AS
v_number_list return_numbers;
BEGIN
SELECT ROWNUM r
BULK COLLECT INTO v_number_list
FROM DUAL
CONNECT BY ROWNUM <= %d;

FOR i IN 1 .. v_number_list.COUNT
LOOP
PIPE ROW (v_number_list(i));
END LOOP;

RETURN;
END;',
$totalRowCount
));
}
}