Skip to content

Stop using LockCol in registrations table#7935

Merged
jprenken merged 5 commits intomainfrom
unlockcol-1
Jan 17, 2025
Merged

Stop using LockCol in registrations table#7935
jprenken merged 5 commits intomainfrom
unlockcol-1

Conversation

@jprenken
Copy link
Copy Markdown
Contributor

@jprenken jprenken commented Jan 11, 2025

Alter the LockCol column to have a default value, so we can omit it from INSERTs.

Part of #7934

@jprenken jprenken changed the base branch from main to updatereg-3 January 13, 2025 18:57
@jprenken jprenken marked this pull request as ready for review January 13, 2025 18:58
@jprenken jprenken requested a review from a team as a code owner January 13, 2025 18:58
@jprenken jprenken requested review from beautifulentropy and removed request for a team January 13, 2025 18:58
@github-actions
Copy link
Copy Markdown
Contributor

@jprenken, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

@letsencrypt letsencrypt deleted a comment from github-actions Bot Jan 13, 2025
@beautifulentropy
Copy link
Copy Markdown
Member

Taking into account the changes in #7911, this generally looks good. I'll take another look once #7911 is merged.

Base automatically changed from updatereg-3 to main January 14, 2025 21:54
@jprenken
Copy link
Copy Markdown
Contributor Author

IN-10959

Copy link
Copy Markdown
Member

@beautifulentropy beautifulentropy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. A note for other reviewers: the first time I looked at this migration the ALTER COLUMN syntax caught me by surprise. I'm used to seeing MODIFY COLUMN from MySQL. But since at least MariaDB 10.2 this has been part of the broader ALTER TABLE functionality

@beautifulentropy beautifulentropy requested review from a team, aarongable and jsha and removed request for a team January 15, 2025 20:16
Copy link
Copy Markdown
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The LockCol that used to be on the certificateStatus table had a DEFAULT 0 before we dropped it, so this makes sense as the correct step towards dropping this column entirely.

@jprenken jprenken merged commit 6f4eb5a into main Jan 17, 2025
@jprenken jprenken deleted the unlockcol-1 branch January 17, 2025 20:41
jprenken added a commit that referenced this pull request Jul 7, 2025
We stopped actively using `LockCol` in the `registrations` table in #7911, and `DEFAULT`ed inserts to 0 in #7935.

Part of #7934
jprenken added a commit that referenced this pull request Jul 8, 2025
We stopped actively using `LockCol` in the `registrations` table in
#7911, and `DEFAULT`ed inserts to 0 in #7935.

Part of #7934
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants