Merge tag '2.12.1' into oracle122

Release [2.12.1](https://github.com/doctrine/dbal/milestone/84) 2.12.1 ====== - Total issues resolved: **2** - Total pull requests resolved: **11** - Total contributors: **7** Documentation,Prepared Statements --------------------------------- - [4424: Mark SQLParserUtils internal](doctrine#4424) thanks to @morozov Packaging --------- - [4416: Update .gitattributes](doctrine#4416) thanks to @bytestream Bug,Cache --------- - [4414: ResultCacheStatement::fetchAllAssociative does not store results in cache](doctrine#4414) thanks to @morozov and @dFayet Deprecation,Prepared Statements ------------------------------- - [4411: Deprecate inappropriate usage of prepared statement parameters](doctrine#4411) thanks to @morozov - [4407: Deprecate colon prefix for prepared statement parameters](doctrine#4407) thanks to @morozov Static Analysis --------------- - [4403: Remove redundant phpstan param from DriverManager::getConnection()](doctrine#4403) thanks to @simPod Bug,Locking,Transactions ------------------------ - [4400: LockMode::NONE should not set WITH (NOLOCK)](doctrine#4400) thanks to @BenMorel Code Style,PHP -------------- - [4398: Update PHP&doctrine#95;CodeSniffer to 3.5.8](doctrine#4398) thanks to @morozov PDO,PHP,Test Suite ------------------ - [4396: Fix php8 mysql mariadb](doctrine#4396) thanks to @greg0ire Documentation ------------- - [4390: Fix headline in the upgrade docs](doctrine#4390) thanks to @jdreesen Documentation,Testing --------------------- - [4356: Testing Guidelines](doctrine#4356) thanks to @morozov # gpg: Signature made Sat Nov 14 21:50:01 2020 # gpg: using DSA key 1BEDEE0A820BC30D858F9F0C2C3A645671828132 # gpg: Can't check signature: No public key # Conflicts: # README.md
Rossmann-IT · Mar 7, 2021 · 42fbc70 · 42fbc70
2 parents 11b6fe8 + adce7a9
commit 42fbc70
Show file tree

Hide file tree

Showing 23 changed files with 370 additions and 54 deletions.
diff --git a/.doctrine-project.json b/.doctrine-project.json
@@ -12,15 +12,21 @@
             "upcoming": true
         },
         {
-            "name": "2.11",
-            "branchName": "2.11.x",
-            "slug": "2.11",
+            "name": "2.12",
+            "branchName": "2.12.x",
+            "slug": "2.12",
             "current": true,
             "aliases": [
                 "current",
                 "stable"
             ]
         },
+        {
+            "name": "2.11",
+            "branchName": "2.11.x",
+            "slug": "2.11",
+            "maintained": false
+        },
         {
             "name": "2.10",
             "branchName": "2.10.x",

diff --git a/.gitattributes b/.gitattributes
@@ -1,4 +1,5 @@
 /.appveyor.yml export-ignore
+/ci export-ignore
 /composer.lock export-ignore
 /docs export-ignore
 /.doctrine-project.json export-ignore

diff --git a/README.md b/README.md
@@ -40,12 +40,12 @@ $table->addUniqueIndex(
 
 ## Original README from doctrine/dbal:
 
-| [Master][Master] | [2.11][2.11] |
+| [Master][Master] | [2.12][2.12] |
 |:----------------:|:----------:|
-| [![Build status][Master image]][Master] | [![Build status][2.11 image]][2.11] |
-| [![GitHub Actions][GA master image]][GA master] | [![GitHub Actions][GA 2.11 image]][GA 2.11] |
-| [![AppVeyor][AppVeyor master image]][AppVeyor master] | [![AppVeyor][AppVeyor 2.11 image]][AppVeyor 2.11] |
-| [![Code Coverage][Coverage image]][CodeCov Master] | [![Code Coverage][Coverage 2.11 image]][CodeCov 2.11] |
+| [![Build status][Master image]][Master] | [![Build status][2.12 image]][2.12] |
+| [![GitHub Actions][GA master image]][GA master] | [![GitHub Actions][GA 2.12 image]][GA 2.12] |
+| [![AppVeyor][AppVeyor master image]][AppVeyor master] | [![AppVeyor][AppVeyor 2.12 image]][AppVeyor 2.12] |
+| [![Code Coverage][Coverage image]][CodeCov Master] | [![Code Coverage][Coverage 2.12 image]][CodeCov 2.12] |
 
 Powerful database abstraction layer with many features for database schema introspection, schema management and PDO abstraction.
 
@@ -64,11 +64,11 @@ Powerful database abstraction layer with many features for database schema intro
   [GA master]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3Amaster
   [GA master image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg
 
-  [2.11 image]: https://img.shields.io/travis/doctrine/dbal/2.11.x.svg?style=flat-square
-  [Coverage 2.11 image]: https://codecov.io/gh/doctrine/dbal/branch/2.11.x/graph/badge.svg
-  [2.11]: https://github.com/doctrine/dbal/tree/2.11.x
-  [CodeCov 2.11]: https://codecov.io/gh/doctrine/dbal/branch/2.11.x
-  [AppVeyor 2.11]: https://ci.appveyor.com/project/doctrine/dbal/branch/2.11.x
-  [AppVeyor 2.11 image]: https://ci.appveyor.com/api/projects/status/i88kitq8qpbm0vie/branch/2.11.x?svg=true
-  [GA 2.11]: https://github.com/doctrine/dbal/actions?query=workflow%3A%22Continuous+Integration%22+branch%3A2.11.x
-  [GA 2.11 image]: https://github.com/doctrine/dbal/workflows/Continuous%20Integration/badge.svg?branch=2.11.x
+  [2.12 image]: https://img.shields.io/travis/doctrine/dbal/2.12.x.svg?style=flat-square
+  [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
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,53 @@
 # Upgrade to 2.12
 
+## Deprecated non-zero based positional parameter keys
+
+The usage of one-based and other non-zero-based keys when binding positional parameters is deprecated.
+
+It is recommended to not use any array keys so that the value of the parameter array complies with the [`list<>`](https://psalm.dev/docs/annotating_code/type_syntax/array_types/) type constraint.
+
+```php
+// This is valid (implicit zero-based parameter indexes)
+$conn->fetchNumeric('SELECT ?, ?', [1, 2]);
+
+// This is invalid (one-based parameter indexes)
+$conn->fetchNumeric('SELECT ?, ?', [1 => 1, 2 => 2]);
+
+// This is invalid (arbitrary parameter indexes)
+$conn->fetchNumeric('SELECT ?, ?', [-31 => 1, 5 => 2]);
+
+// This is invalid (non-sequential parameter indexes)
+$conn->fetchNumeric('SELECT ?, ?', [0 => 1, 3 => 2]);
+```
+
+## Deprecated skipping prepared statement parameters
+
+Some underlying drivers currently allow skipping prepared statement parameters. For instance:
+
+```php
+$conn->fetchOne('SELECT ?');
+// NULL
+```
+
+This behavior should not be relied upon and may change in future versions.
+
+## Deprecated colon prefix for prepared statement parameters
+
+The usage of the colon prefix when binding named parameters is deprecated.
+
+```php
+$sql  = 'SELECT * FROM users WHERE name = :name OR username = :username';
+$stmt = $conn->prepare($sql);
+
+// The usage of the leading colon is deprecated
+$stmt->bindValue(':name', $name);
+
+// Only the parameter name should be passed
+$stmt->bindValue('username', $username);
+
+$stmt->execute();
+```
+
 ## PDO signature changes with php 8
 
 In php 8.0, the method signatures of two PDO classes which are extended by DBAL have changed. This affects the following classes:
@@ -76,7 +124,7 @@ The following PDO-related classes outside of the PDO namespace have been depreca
 3. `prefersSequences()`.
 4. `supportsForeignKeyOnUpdate()`.
 
-##`ServerInfoAwareConnection::requiresQueryForServerVersion()` is deprecated.
+## `ServerInfoAwareConnection::requiresQueryForServerVersion()` is deprecated.
 
 The `ServerInfoAwareConnection::requiresQueryForServerVersion()` method has been deprecated as an implementation detail which is the same for almost all supported drivers.
 

diff --git a/composer.lock b/composer.lock
diff --git a/docs/en/reference/data-retrieval-and-manipulation.rst b/docs/en/reference/data-retrieval-and-manipulation.rst
@@ -91,9 +91,11 @@ are then replaced by their actual values in a second step (execute).
     $stmt->bindValue(1, $id);
     $stmt->execute();
 
-Placeholders in prepared statements are either simple positional question marks (?) or named labels starting with
-a double-colon (:name1). You cannot mix the positional and the named approach. The approach
-using question marks is called positional, because the values are bound in order from left to right
+Placeholders in prepared statements are either simple positional question marks (``?``) or named labels starting with
+a colon (e.g. ``:name1``). You cannot mix the positional and the named approach. You have to bind a parameter
+to each placeholder.
+
+The approach using question marks is called positional, because the values are bound in order from left to right
 to any question mark found in the previously prepared SQL query. That is why you specify the
 position of the variable to bind into the ``bindValue()`` method:
 

diff --git a/docs/en/reference/testing.rst b/docs/en/reference/testing.rst
@@ -0,0 +1,123 @@
+Testing Guidelines
+===================
+
+To ensure high quality, all components of the Doctrine DBAL library are extensively covered with tests.
+
+Having the code covered with tests and running all tests against each individual code change helps prevent
+breakages of the library logic when its code changes.
+
+Additionally, when code changes are accompanied by new tests, the tests:
+
+1. Help understand what problem the given code change is trying to solve.
+2. Make sure that the problem being solved needs to be solved in the DBAL.
+3. Document the proper usage of the DBAL APIs.
+
+Requirements
+------------
+
+1. Each pull request that adds new or changes the existing logic must have tests.
+
+   .. note::
+
+       Modifications to the keyword lists under the ``Doctrine\DBAL\Platforms\Keywords`` namespace
+       don't have to be covered with tests.
+
+2. The test that covers certain logic must fail without this logic implemented.
+
+Types of Tests
+--------------
+
+Doctrine DBAL primarily uses unit and integration tests.
+
+Unit Tests
+~~~~~~~~~~
+
+Unit tests are meant to cover the logic of a given unit (e.g. a class or a method) including the logic
+of its interaction with other units. In this case, the other units could be mocked.
+
+Unit tests are most welcomed for testing the logic that the DBAL itself defines (e.g. logging, caching, data types).
+
+In this case, the DBAL is the source of truth about what this logic is and the test plays the role of its description.
+
+Integration Tests
+~~~~~~~~~~~~~~~~~
+
+Integration (a.k.a. functional) tests are required when the behavior under test is dictated by the logic
+defined outside of the DBAL. It could be:
+
+- The underlying database platform.
+- The underlying database driver.
+- SQL syntax and the standard as such.
+
+It is important to have integration tests for the cases above. Unlike unit tests, they make the external components
+the source of truth and help make sure that the logic implemented in the DBAL is correct even if the external components
+change (e.g. a new version of a database platform is supported).
+
+When are Integration Tests not Required?
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Some cases cannot be reproduced with the existing integration testing suite. It could be the scenarios that involve
+multiple concurrent database connections, transactions, locking, performance-related issues, etc.
+
+In such cases, it is still important that a pull request fixing the issues is accompanied by a free-form reproducer
+that demonstrates the issue being fixed.
+
+Recommendations on Writing Tests
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Tests in Doctrine DBAL are located under the ``tests`` directory and implemented on top of PHPUnit. Use its
+`documentation <https://phpunit.de/documentation.html>`_ to get started.
+
+Writing Integration Tests
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Integration tests are located under the ``tests/Doctrine/Tests/DBAL/Functional`` directory. Unlike unit tests,
+they require a real database connection to test their logic against.
+
+It is recommended to use ``Doctrine\DBAL\Tests\FunctionalTestCase`` as the base class for integration tests.
+Based on the configuration, it will automatically create and connect to the test database.
+
+Data Fixtures in Integration Tests
+++++++++++++++++++++++++++++++++++
+
+To test selecting and fetching data from the database, the test may create the necessary schema and populate it
+with the test data. To create database tables, instead of checking if the table exists, it is recommended
+to use ``AbstractSchemaManager::dropAndCreateTable()``. This way, the table will be dropped and created every time
+providing better isolation between the test runs.
+
+Testing Different Database Platforms
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Although most of the issues are originally discovered on a specific database platform,
+the integration tests for all issues should be implemented by default at the database abstraction level
+and run against all the platforms that support the API being tested.
+
+This allows us to ensure that the same scenario that was found failing on one platform also works on others. Or otherwise,
+the same issue could be reproduced on the platforms where it wasn't originally tested.
+
+If the newly added test fails on other platforms, and fixing it is out of the scope, the test can be explicitly marked
+as incomplete which will identify the issue.
+
+Examples of such tests could be found under the ``Doctrine\Tests\DBAL\Functional\Platform`` namespace.
+
+Using Unit and Integration Tests Together
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+For example, the ``AbstractPlatform::modifyLimitQuery()`` method has both unit and integration tests.
+
+1. Unit test cases for each platform (``Doctrine\Tests\DBAL\Platforms\*PlatformTest``) have a test that calls
+   ``$platform->modifyLimitQuery()`` and asserts that the resulting SQL looks as expected.
+   These tests cannot guarantee that the generated SQL is valid syntactically and semantically but they guarantee
+   that the code works as designed. They provide fast feedback because they don't require a database connection
+   and can test all platforms in a single test suite run.
+2. There is an integration test ``Doctrine\Tests\DBAL\Functional\ModifyLimitQueryTest`` which calls
+   ``$platform->modifyLimitQuery()`` and executes the generated queries on a real database to which the test suite
+   is connected. This test guarantees that the generated queries are valid but it's much slower and works
+   only with one database at a time.
+
+As you can see, both approaches have their strengths and weaknesses and can complement each other.
+
+.. warning::
+
+    Do not mix the unit and the integration approaches in one test. Each of the approaches has its area of application
+    and purpose. Mixing them makes it harder to identify the reason and the impact of a failing mixed-type test.
diff --git a/docs/en/sidebar.rst b/docs/en/sidebar.rst
@@ -20,5 +20,6 @@
     reference/caching
     reference/known-vendor-issues
     reference/upgrading
+    reference/testing
 
     explanation/implicit-indexes
diff --git a/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php b/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php
@@ -242,7 +242,9 @@ public function fetchAllNumeric(): array
             $data = $this->statement->fetchAll(FetchMode::ASSOCIATIVE);
         }
 
-        $this->store($data);
+        $this->data = $data;
+
+        $this->saveToCache();
 
         return array_map('array_values', $data);
     }
@@ -258,7 +260,9 @@ public function fetchAllAssociative(): array
             $data = $this->statement->fetchAll(FetchMode::ASSOCIATIVE);
         }
 
-        $this->store($data);
+        $this->data = $data;
+
+        $this->saveToCache();
 
         return $data;
     }
@@ -322,14 +326,6 @@ private function doFetch()
         return false;
     }
 
-    /**
-     * @param array<int,array<string,mixed>> $data
-     */
-    private function store(array $data): void
-    {
-        $this->data = $data;
-    }
-
     private function saveToCache(): void
     {
         if ($this->data === null) {

diff --git a/lib/Doctrine/DBAL/Driver/PDOOracle/Driver.php b/lib/Doctrine/DBAL/Driver/PDOOracle/Driver.php
@@ -10,11 +10,6 @@
 /**
  * PDO Oracle driver.
  *
- * WARNING: This driver gives us segfaults in our testsuites on CLOB and other
- * stuff. PDO Oracle is not maintained by Oracle or anyone in the PHP community,
- * which leads us to the recommendation to use the "oci8" driver to connect
- * to Oracle instead.
- *
  * @deprecated Use {@link PDO\OCI\Driver} instead.
  */
 class Driver extends AbstractOracleDriver

diff --git a/lib/Doctrine/DBAL/DriverManager.php b/lib/Doctrine/DBAL/DriverManager.php
@@ -119,7 +119,6 @@ private function __construct()
      *
      * @throws Exception
      *
-     * @phpstan-param mixed[] $params
      * @psalm-return ($params is array{wrapperClass:mixed} ? T : Connection)
      * @template T of Connection
      */

diff --git a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php
@@ -52,7 +52,7 @@ public function appendLockHint($fromClause, $lockMode)
     {
         switch (true) {
             case $lockMode === LockMode::NONE:
-                return $fromClause . ' WITH (NOLOCK)';
+                return $fromClause;
 
             case $lockMode === LockMode::PESSIMISTIC_READ:
                 return $fromClause . ' WITH (UPDLOCK)';

diff --git a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
@@ -1574,7 +1574,7 @@ public function appendLockHint($fromClause, $lockMode)
     {
         switch (true) {
             case $lockMode === LockMode::NONE:
-                return $fromClause . ' WITH (NOLOCK)';
+                return $fromClause;
 
             case $lockMode === LockMode::PESSIMISTIC_READ:
                 return $fromClause . ' WITH (HOLDLOCK, ROWLOCK)';

diff --git a/lib/Doctrine/DBAL/SQLParserUtils.php b/lib/Doctrine/DBAL/SQLParserUtils.php
@@ -26,6 +26,8 @@
 
 /**
  * Utility class that parses sql statements with regard to types and parameters.
+ *
+ * @internal
  */
 class SQLParserUtils
 {