Merge pull request #4581 from amenning/fix-oracle-truncated-fetch

Fix truncated oracle fetch result

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
+        ));
+    }
+}

src/Driver/OCI8/Result.php

@@ -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;
@@ -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();
+        }
     }
 
     /**

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.');
+    }
+
+    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
+        ));
+    }
+}