Database primary keys can exceed maximum integer storeable (has actually occurred, in watchdog)

🐛 Schema::changeField() has bug when changing regular serial field to big serial field Fixed was merged for D10 and I just provided the backport for D9.5 I think this is now unblocked.

Upgraded to work with Drupal 10.1.x. The lines from patch #49 regarding core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php now seems included in another file core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php.

I think I also addressed comment .3 from #48 🐛 Database primary keys can exceed maximum integer storeable (has actually occurred, in watchdog) Fixed

The last submitted patch, 66: 3081144-66.patch, failed testing. View results →

Thanks for this input @dagmar, I think we could make this .install function even shorter using some inspiration from issue #3191391.

Let's see if this breaks the tests or not.

I updated the issue summary to describe the proposed solution.

+++ b/core/modules/dblog/tests/src/Functional/UpdatePathTest.php
@@ -0,0 +1,79 @@
+    $insert = Database::getConnection()->insert('watchdog');
+
+    $connection = $this->prophesize(Connection::class);
+    $connection->getTarget()->willReturn(DbLog::DEDICATED_DBLOG_CONNECTION_TARGET);
+    $connection->insert('watchdog')->willReturn(new class ($insert) {
+
+      /**
+       * The decorated query object.
+       *
+       * @var \Drupal\Core\Database\Query\Insert
+       */
+      private $decorated;
+
+      /**
+       * Constructor for this anonymous class.
+       *
+       * @param \Drupal\Core\Database\Query\Insert $decorated
+       *   The decorated query object.
+       */
+      public function __construct(Insert $decorated) {
+        $this->decorated = $decorated;
+      }
+
+      /**
+       * Overrides \Drupal\Core\Database\Query\Insert::fields().
+       *
+       * Purposefully sets the 'wid' column of the query to a value which will
+       * overflow the 32-bit integer limit.
+       */
+      public function fields(array $fields, array $values = []) {
+        $fields['wid'] = PHP_INT_MAX;
+        return $this->decorated->fields($fields, $values);
+      }
+
+    });
+    $this->container->set('database', $connection->reveal());
+    $this->generateLogEntries(1);

Can we replace this part with:
- insert row into table with a value for wid that is bigger than the regular maximum integer value.
- test that the row exists in the table with the correct value for the wid field
- insert another row without a value for wid. With the auto-increment functionality it should get the previous wid value plus 1.
- test that the new row exists in the table with the correct value for the wid field.

added updated patch and addressed #71

1. +++ b/core/modules/dblog/tests/src/Functional/UpdatePathTest.php
   @@ -0,0 +1,78 @@
   +    $connection = $this->prophesize(Connection::class);
   

   We do not need to prophesize the database connection. Just use the regular one

2. +++ b/core/modules/dblog/tests/src/Functional/UpdatePathTest.php
   @@ -0,0 +1,78 @@
   +      $fields['wid'] = PHP_INT_MAX;
   

   Can we change the value of PHP_MAX_INT with its value times 1000. That will be 2147483647000.

All we need to do is to test that we can insert a value for the wid field that is bigger than 2147483647 and that for values bigger than that the auto increment functionality works. No prophesize functionality needed.

Addressed #73 The updated code inserts a row with a big value for wid and another row without a value for wid. It then tests that the first row exists with the expected value for wid and that the second row exists with the expected auto-incremented value for wid.

1. +++ b/core/modules/dblog/tests/src/Functional/UpdatePathTest.php
   @@ -0,0 +1,63 @@
   +    $insert->fields(['wid' => PHP_INT_MAX]);
   ...
   +      ->condition('wid', PHP_INT_MAX)
   ...
   +      ->condition('wid', PHP_INT_MAX + 1)
   

   Can you replace PHP_INT_MAX with 2147483647000.

2. +++ b/core/modules/dblog/tests/src/Functional/UpdatePathTest.php
   @@ -0,0 +1,63 @@
   +    $insert = Database::getConnection()->insert('watchdog');
   ...
   +    $insert = Database::getConnection()->insert('watchdog');
   ...
   +    $result = Database::getConnection()->select('watchdog')
   ...
   +    $result = Database::getConnection()->select('watchdog')
   

   Can we use the variable $connection. Please add: $connection = Database::getConnection();

Addressed #75 comment.
Thanks

The last submitted patch, 76: 3081144-76.patch, failed testing. View results →

@Akram Khan and @pratik: You are supposed to run the test you are working on your local machine. To make sure that it passes. Now it is not.

I've adjusted the patch so the test don't fail. The dblog table has a lot of not-null fields, so we need to fill these to insert the log.

I've also removed the use cases for FakeLogEntries and ProphecyTrait, we still pass the test and we don't need to alter the baseline anymore.

All code changes look good to me.
I have updated the IS.
For me it is RTBC.

Automatically closed - issue fixed for 2 weeks with no activity.