Reviewed & tested by the community
Project:
Drupal core
Version:
main
Component:
database system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
12 Nov 2025 at 19:31 UTC
Updated:
20 Sep 2026 at 15:38 UTC
Jump to comment: Most recent, Most recent file
Show commands
changes
Comments
Comment #3
mondrakeComment #4
mondrakeComment #5
mondrakeComment #7
mondrakeComment #8
mondrakeComment #9
mondrakeNeeds some tuning. On that.
Comment #10
mondrakeComment #11
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #12
mondrakeComment #13
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #14
amateescu commentedFound a few issues in the MR, some of them blocking, but in general it's very hard to review such a large change. How about splitting it into conversions for test and non-test modules.
Comment #15
mondrake#14 yes, good idea, will do
Comment #16
mondrakeComment #19
mondrakeApplied the suggestions and reduced scope of the MR here to converting schemata of regular, non-test, modules.
Comment #20
mondrakeComment #21
daffie commentedComment #22
mondrakeThanks
Comment #23
daffie commentedLast nitpick, for the rest is it RTBC for me.
Comment #24
mondrakeComment #25
daffie commentedLooks good to me.
All my remarks are addressed.
For me it is RTBC.
Comment #26
amateescu commentedThe new MR looks much better and easier to review, thanks! Left a few review points, one of them being that we're missing an update hook for the
widcolumn.Comment #27
mondrakeadded update function + update path test, reverted ModuleInstaller changes to be postponed to the follow up.
Comment #28
mondrakethe update path test for dblog is green on all dbs. mysqli has some test failures but seem unrelated to me.
Comment #29
mondrakecouple things to check on the update path
Comment #30
mondrakeComment #31
meeni_dhobale commentedTested MR !16910 on
main/PHP 8.5 against bothpdo_mysqlandmysqliit applies cleanly, all tests pass, and I verified via DDL diff (old site + patch +updatedbvs. fresh install) that no module besidesdblogneeds an update hook.Also confirmed
dblog_update_11500's unsigned fix addresses a real, live schema discrepancy (checked against the legacy driver).Looks good to me.
Comment #32
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #33
mondrakerebased
Comment #34
mondrakeComment #35
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #36
mondrakeRebased.
Comment #37
amateescu commentedReviewed again and posted a few more comments on the MR.
Comment #38
mondrakelet's fix based on those two comments. on that.
Comment #39
mondrakethanks, made the changes suggested.
Comment #40
amateescu commentedLooks great to me now! We should really try and get this into 11.5 so we can showcase the new API and set the right example for new code.
Comment #41
mondrakeMR!16910 is the one RTBCed.
Comment #42
larowlanLeft some comments but keeping at RTBC
Comment #43
mondrakeComment #44
mondrakeAddressed @larowlan input, and reverted the accidental removal of a couple of asserts.
Comment #45
amateescu commentedThe recent changes look good to me :)
Comment #46
meeni_dhobale commentedI have retested the new current changes and it looks good to me. MR applies cleanly and all test passes clean. Tried to find out the edge cases where it might fail but didn't find any at my end. It looks good to me as well. RTBC.
Comment #47
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #48
mondrakeRebased and added a couple of more validations for duplicate table in schema and duplicate column in table, that we missed earlier, as found in https://git.drupalcode.org/project/drupal/-/merge_requests/16823#note_23...
Comment #49
amateescu commentedStill looking good :)