diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 35f1d6645a0..d049b95c7b2 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -52,3 +52,4 @@ jobs: uses: docker://vimeo/psalm-github-actions:4.6.4 with: composer_require_dev: true + args: --shepherd diff --git a/README.md b/README.md index ae4467a5f12..f936180712e 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,11 @@ # Doctrine DBAL -| [4.0-dev][4.0] | [3.0][3.0] | [2.12][2.12] | +| [4.0-dev][4.0] | [3.0][3.0] | [2.13][2.13] | |:----------------:|:----------:|:----------:| -| [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.0 image]][GA 3.0] | [![GitHub Actions][GA 2.12 image]][GA 2.12] | -| [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.0 image]][AppVeyor 3.0] | [![AppVeyor][AppVeyor 2.12 image]][AppVeyor 2.12] | -| [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.0 image]][CodeCov 3.0] | [![Code Coverage][Coverage 2.12 image]][CodeCov 2.12] | +| [![GitHub Actions][GA 4.0 image]][GA 4.0] | [![GitHub Actions][GA 3.0 image]][GA 3.0] | [![GitHub Actions][GA 2.13 image]][GA 2.13] | +| [![AppVeyor][AppVeyor 4.0 image]][AppVeyor 4.0] | [![AppVeyor][AppVeyor 3.0 image]][AppVeyor 3.0] | [![AppVeyor][AppVeyor 2.13 image]][AppVeyor 2.13] | +| [![Code Coverage][Coverage image]][CodeCov 4.0] | [![Code Coverage][Coverage 3.0 image]][CodeCov 3.0] | [![Code Coverage][Coverage 2.13 image]][CodeCov 2.13] | +| N/A | [![Code Coverage][TypeCov 3.1 image]][TypeCov 3.1] | N/A | Powerful database abstraction layer with many features for database schema introspection, schema management and PDO abstraction. @@ -30,10 +31,12 @@ Powerful database abstraction layer with many features for database schema intro [GA 3.0]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A3.0.x [GA 3.0 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=3.0.x - [Coverage 2.12 image]: https://codecov.io/gh/doctrine/dbal/branch/2.12.x/graph/badge.svg - [2.12]: https://github.com/doctrine/dbal/tree/2.12.x - [CodeCov 2.12]: https://codecov.io/gh/doctrine/dbal/branch/2.12.x - [AppVeyor 2.12]: https://ci.appveyor.com/project/doctrine/dbal/branch/2.12.x - [AppVeyor 2.12 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.12.x?svg=true - [GA 2.12]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.12.x - [GA 2.12 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.12.x + [Coverage 2.13 image]: https://codecov.io/gh/doctrine/dbal/branch/2.13.x/graph/badge.svg + [2.13]: https://github.com/doctrine/dbal/tree/2.13.x + [CodeCov 2.13]: https://codecov.io/gh/doctrine/dbal/branch/2.13.x + [AppVeyor 2.13]: https://ci.appveyor.com/project/doctrine/dbal/branch/2.13.x + [AppVeyor 2.13 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.13.x?svg=true + [GA 2.13]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.13.x + [GA 2.13 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.13.x + [TypeCov 3.1]: https://shepherd.dev/github/doctrine/dbal + [TypeCov 3.1 image]: https://shepherd.dev/github/doctrine/dbal/coverage.svg diff --git a/psalm.xml.dist b/psalm.xml.dist index 82f37d7b94c..2286ac5aa07 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -54,6 +54,11 @@ See https://github.com/doctrine/dbal/pull/4317 --> + + + diff --git a/src/Driver/OCI8/Result.php b/src/Driver/OCI8/Result.php index 5cbf007a247..8f77da75905 100644 --- a/src/Driver/OCI8/Result.php +++ b/src/Driver/OCI8/Result.php @@ -4,10 +4,13 @@ namespace Doctrine\DBAL\Driver\OCI8; +use Doctrine\DBAL\Driver\Exception; use Doctrine\DBAL\Driver\FetchUtils; +use Doctrine\DBAL\Driver\OCI8\Exception\Error; use Doctrine\DBAL\Driver\Result as ResultInterface; use function oci_cancel; +use function oci_error; use function oci_fetch_all; use function oci_fetch_array; use function oci_num_fields; @@ -112,13 +115,18 @@ public function free(): void /** * @return mixed|false + * + * @throws Exception */ private function fetch(int $mode) { - return oci_fetch_array( - $this->statement, - $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS - ); + $result = oci_fetch_array($this->statement, $mode | OCI_RETURN_NULLS | OCI_RETURN_LOBS); + + if ($result === false && oci_error($this->statement) !== false) { + throw Error::new($this->statement); + } + + return $result; } /** diff --git a/src/Statement.php b/src/Statement.php index 3f3ac12ba45..731e8d10672 100644 --- a/src/Statement.php +++ b/src/Statement.php @@ -8,6 +8,7 @@ use Doctrine\DBAL\Driver\Statement as DriverStatement; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Types\Type; +use Doctrine\Deprecations\Deprecation; use function is_string; @@ -156,10 +157,18 @@ public function bindParam($param, &$variable, int $type = ParameterType::STRING, /** * {@inheritDoc} * + * @deprecated Statement::execute() is deprecated, use Statement::executeQuery() or executeStatement() instead + * * @throws Exception */ public function execute(?array $params = null): Result { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4580', + 'Statement::execute() is deprecated, use Statement::executeQuery() or Statement::executeStatement() instead' + ); + if ($params !== null) { $this->params = $params; } @@ -179,6 +188,38 @@ public function execute(?array $params = null): Result } } + /** + * Executes the statement with the currently bound parameters and return result. + * + * @param mixed[] $params + * + * @throws Exception + */ + public function executeQuery(array $params = []): Result + { + if ($params === []) { + $params = null; // Workaround as long execute() exists and used internally. + } + + return $this->execute($params); + } + + /** + * Executes the statement with the currently bound parameters and return affected rows. + * + * @param mixed[] $params + * + * @throws Exception + */ + public function executeStatement(array $params = []): int + { + if ($params === []) { + $params = null; // Workaround as long execute() exists and used internally. + } + + return $this->execute($params)->rowCount(); + } + /** * Gets the wrapped driver statement. */ diff --git a/tests/Functional/Driver/OCI8/ResultTest.php b/tests/Functional/Driver/OCI8/ResultTest.php new file mode 100644 index 00000000000..e290dd763ff --- /dev/null +++ b/tests/Functional/Driver/OCI8/ResultTest.php @@ -0,0 +1,170 @@ + + */ + 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('DROP FUNCTION test_oracle_fetch_failure'); + $this->connection->executeQuery('DROP TYPE return_numbers'); + + 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) { + // prevent the PHPUnit error handler from handling the warnings that oci_*() functions may trigger + $this->iniSet('error_reporting', (string) (E_ALL & ~E_WARNING)); + + $this->expectException(DriverException::class); + $this->expectExceptionCode(4068); + } + + // 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) + $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 ($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; + } + + self::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 + )); + } +} diff --git a/tests/Functional/StatementTest.php b/tests/Functional/StatementTest.php index 3141d7f44a7..b7d83f356ae 100644 --- a/tests/Functional/StatementTest.php +++ b/tests/Functional/StatementTest.php @@ -361,4 +361,42 @@ public function testExecWithRedundantParameters(): void $stmt->execute([null]); } + + public function testExecuteQuery(): void + { + $platform = $this->connection->getDatabasePlatform(); + $query = $platform->getDummySelectSQL(); + $result = $this->connection->prepare($query)->executeQuery()->fetchOne(); + + self::assertEquals(1, $result); + } + + public function testExecuteQueryWithParams(): void + { + $this->connection->insert('stmt_test', ['id' => 1]); + + $query = 'SELECT id FROM stmt_test WHERE id = ?'; + $result = $this->connection->prepare($query)->executeQuery([1])->fetchOne(); + + self::assertEquals(1, $result); + } + + public function testExecuteStatement(): void + { + $this->connection->insert('stmt_test', ['id' => 1]); + + $query = 'UPDATE stmt_test SET name = ? WHERE id = 1'; + $stmt = $this->connection->prepare($query); + + $stmt->bindValue(1, 'bar'); + + $result = $stmt->executeStatement(); + + self::assertEquals(1, $result); + + $query = 'UPDATE stmt_test SET name = ? WHERE id = ?'; + $result = $this->connection->prepare($query)->executeStatement(['foo', 1]); + + self::assertEquals(1, $result); + } } diff --git a/tests/Query/QueryBuilderTest.php b/tests/Query/QueryBuilderTest.php index c14a8ef7fa0..9a15e4309af 100644 --- a/tests/Query/QueryBuilderTest.php +++ b/tests/Query/QueryBuilderTest.php @@ -849,6 +849,23 @@ public function testJoinWithNonUniqueAliasThrowsException(): void $qb->getSQL(); } + public function testExecuteSelect(): void + { + $qb = new QueryBuilder($this->conn); + + $this->conn + ->expects(self::any()) + ->method('executeQuery') + ->willReturn($this->createMock(Result::class)); + + $result = $qb + ->select('id') + ->from('foo') + ->execute(); + + self::assertInstanceOf(Result::class, $result); + } + /** * @param list|array $parameters * @param array|array $parameterTypes