Problem/Motivation

In #1411074: Allow kernel tests to share the test environment we are working on resetting the database whenever a new test method is executed. This requires, among other things, to reset serial IDs, which requires a different logic for each DB driver.

Proposed resolution

Add a new DB connection method to reset serial IDs.

Remaining tasks

  • Validate the proposed solution
  • Provide a merge request
  • Reviews

API changes

None, only additions.

Release notes snippet

TBD

Issue fork drupal-3617897

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

plach created an issue. See original summary.

plach opened merge request !16759

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Assigned: plach » Unassigned
Issue summary: View changes
Status: Active » Needs review
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Issue summary: View changes
daffie’s picture

daffie commented
Category: Feature request » Bug report
Status: Needs review » Needs work
Issue tags: -API addition

This is to me not a feature request, but just a plain bug. When I truncate a table, I would expect that the value of the primary key would start at 1 again. No new method is needed, just a bugfix and testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

Thanks for the feedback! I'm a bit concerned about BC: if this wasn't reported before, couldn't people be relying on the current non-MySQL behaviors?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

Maybe a change record would be enough, if we only merge this in D12.

daffie’s picture

daffie commented

During the running of a site in production, you will not be calling Connection::truncate() on any table. It is a special operation. And yes, any bug on which code is relaying on to be there, will result in a BC break when you fix the bug. That has not stopped us fixing other bugs. Maybe we can add an option to get the original functionality back, but it default value should be to reset the serial id. We should add a CR for this bug fix.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

Pushed an update resetting serial IDs by default, unless within a transaction or reset skipped explicitly.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Needs review

plach opened merge request !17096

oily changed the visibility of the branch 3617897-add-a-new to hidden.

ironnuts’s picture

ironnuts commented

Re: #8 and #9, #8 states:

but it default value should be to reset the serial id. We should add a CR for this bug fix.

plach, you state at #9 that your commit resets serial ID's by default. So we need a CR, right?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

So we do not need a CR?

I'd say we do need a CR anyway, because we are adding a new option. There we can mention the behavior change, to ensure it's not missed.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Issue tags: +Needs issue summary update, +Needs change record

We also need an issue summary update, but I'd like to get some feedback on the MR before working on those.

ironnuts’s picture

ironnuts commented

Re: #15 Grazie. Comprendo.

daffie’s picture

daffie commented
Status: Needs review » Needs work

I have a single remark on the PR.
The gitlab CI pipeline also needs to run for all the supported databases (MySQL, MariaDB, PostgreSQL and SQLite).