-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
ref(grouping): Add schema_version
to GroupHashMetadata
table
#85300
base: master
Are you sure you want to change the base?
ref(grouping): Add schema_version
to GroupHashMetadata
table
#85300
Conversation
bd7110d
to
d1779e0
Compare
f7dca7f
to
c217cae
Compare
d1779e0
to
558d105
Compare
c217cae
to
4091f58
Compare
4091f58
to
467c959
Compare
467c959
to
5616bf8
Compare
@@ -53,6 +76,9 @@ class GroupHashMetadata(Model): | |||
# When the grouphash was created. Will be null for grouphashes created before we started | |||
# collecting metadata. | |||
date_added = models.DateTimeField(default=timezone.now, null=True) | |||
# The version of the metadata schema which produced the data. Useful for backfilling when we add | |||
# to or change the data we collect and want to update existing records. | |||
schema_version = models.CharField(null=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to remove the null once all records are populated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is probably not worth it as we'd have to be careful about the migration https://develop.sentry.dev/backend/application-domains/database-migrations/#adding-constraints-to-columns-including-not-null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. It'd be nice if postgres had some sort of progressively-non-null designation we could set - so that the constraint could be checked as records are touched in the course of normal operations - but AFAIK no such thing exists, so we'll probably have to just live with the nullability. (This goes for pretty much all the fields in the table.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migration DDL looks good.
5616bf8
to
8eea882
Compare
This PR has a migration; here is the generated SQL for --
-- Add field schema_version to grouphashmetadata
--
ALTER TABLE "sentry_grouphashmetadata" ADD COLUMN "schema_version" varchar NULL; |
8eea882
to
f0a33e7
Compare
f0a33e7
to
4e9e8e6
Compare
As we change or add to the data we store in grouphash metadata, we want to be able to update data for grouphashes created before a given change was made. In order to do that, we need to keep track of which version of the grouphash metadata schema was active when a given grouphash's data was collected. This therefore adds a
schema_version
field to the table, and populates it for newGroupHashMetadata
records.