Skip to content

Database: Add primary key to settings table #108569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Database: Add primary key to settings table #108569

wants to merge 1 commit into from

Conversation

inf0rmer
Copy link
Contributor

What is this feature?

Several tables are missing primary keys, with setting being one of those. This PR adds a primary key to that table based on an already existing unique constraint on (section, key).

Why do we need this feature?

Having primary keys is a good idea, but a customer is blocked from upgrading to newer versions of Grafana due to a new internal policy that mandates SQL tables have primary keys.

Who is this feature for?

Database Administrators

Which issue(s) does this PR fix?:

Part of https://github.com/grafana/search-and-storage-team/issues/393

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@inf0rmer inf0rmer self-assigned this Jul 24, 2025
@inf0rmer inf0rmer marked this pull request as ready for review July 24, 2025 09:30
@inf0rmer inf0rmer requested a review from a team as a code owner July 24, 2025 09:30
@inf0rmer inf0rmer requested review from pstibrany and removed request for a team July 24, 2025 09:30
@github-actions github-actions bot added this to the 12.1.x milestone Jul 24, 2025
@inf0rmer inf0rmer requested a review from hairyhenderson July 24, 2025 09:31
@inf0rmer inf0rmer changed the title feat: add primary key to settings table Database: Add primary key to settings table Jul 24, 2025
@pstibrany
Copy link
Contributor

This table is created in Grafana enterprise. I would suggest to put the migration there as well: https://github.com/grafana/grafana-enterprise/blob/02221e9cbbc97fd34f5ba3f68441510640ea0a84/src/pkg/extensions/settings/settingsprovider/database_mig.go

@pstibrany
Copy link
Contributor

I was going to suggest to make corresponding changes to XORM tags, but I can't find a way to specify multi-column PK in Xorm tags. Furthermore we create SQL statements to update rows manually in https://github.com/grafana/grafana-enterprise/blob/a1bf7752ba976045eadb1cb79a03ee7fdb8fdb89/src/pkg/extensions/settings/settingsprovider/database.go#L30 so it won't make a difference.

@inf0rmer
Copy link
Contributor Author

This table is created in Grafana enterprise. I would suggest to put the migration there as well: https://github.com/grafana/grafana-enterprise/blob/02221e9cbbc97fd34f5ba3f68441510640ea0a84/src/pkg/extensions/settings/settingsprovider/database_mig.go

Maybe add it only in enterprise?

@inf0rmer
Copy link
Contributor Author

Closing this and opening the PR in enterprise.

@inf0rmer inf0rmer closed this Jul 29, 2025
@inf0rmer
Copy link
Contributor Author

Superseded by https://github.com/grafana/grafana-enterprise/pull/9198

@inf0rmer inf0rmer deleted the feat/settings-table-add-primary-key branch July 29, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pstibrany pstibrany Awaiting requested review from pstibrany pstibrany is a code owner automatically assigned from grafana/grafana-search-and-storage

@hairyhenderson hairyhenderson Awaiting requested review from hairyhenderson

Assignees

@inf0rmer inf0rmer

Labels
add to changelog area/backend/db/migration area/backend
Projects
None yet
Milestone
12.1.x
Development

Successfully merging this pull request may close these issues.
2 participants
@inf0rmer @pstibrany