Schema::changeField() has bug when changing regular serial field to big serial field

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I think that we for PostgreSQL first need to remove the sequence, than change the field from an integer to a big integer field. After that is done can a new sequence be created. This can be done in the method changeField(). In that method we are already doing the same with constraints on the field. Just before removing the contraints on the field remove the sequence.

I've changed the varchar_acsii type to text, which seems to be working on my end.

Changed the test with some help from @daffie, this should be a bit better to understand.

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

+++ b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
@@ -910,6 +909,14 @@ public function changeField($table, $field, $field_new, $spec, $new_keys = []) {
+      // We need to add CASCADE otherwise we cannot drop the sequence because
+      // the table depends on it.

Just for my understanding (I've not used postgres for about 15 years) can you elaborate on the type of objects that this will remove in Drupal?

Does the table gets removed? We don't do foreign keys so I assume it doesn't spider out to other tables.

@larowlan: The cascade option is necessary, because a sequence in PostgreSQL can have dependencies behind it like a constraint that the value cannot be lower then zero. When you delete the sequence, the constraint also needs to be deleted. The alternative is to delete the constraint first and then delete the sequence. The last option is more work and more that can go wrong.
The text in the comment is about the sequence is preventing the altering of the table. We should change the word "drop" to "alter" in the comment. Hopefully this can be done on commit.

Thanks daffie that makes sense and resolves my concern, I'll look at this tomorrow morning

We're in commit freeze so I will wait for that to finish before committing, but this is top of my list

Committed to 10.1.x and 10.0.x.

Needs reroll for backport to 9.5.x, the test changes don't apply.

Rerolled for Drupal 9.5.x.

core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php has a lot of new lines in D10 than D9.5. I hope this doesn't break anything...

The testbot is failing for PostgreSQL.

Thanks @dagmar for the reroll. The function getPrefix, does not exist in D9.5, because this function replaces tablePrefix(). So in this case for Drupal 9.5 we need to revert back to using tablePrefix().

I could not create a interdiff unfortunately, but I only changed the function getPrefix to tablePrefix($table) in pgsql/Schema / getSequenceName()

This issue relies on code from #3257201: Create the new method Drupal\Core\Database\Connection::getPrefix() and deprecate Drupal\Core\Database\Connection::prefixTable($table) → so we have to revert it from 10.0.x as that issue is only in 10.1.x.

No backport needed any more

On 10.0.x prior to the revert if you run core/tests/Drupal/KernelTests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php you will get an error like:

Error : Call to undefined method Drupal\pgsql\Driver\Database\pgsql\Connection::getPrefix()
 /Volumes/dev/sites/drupal8alt.dev/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php:1086

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