Skip to content

Commit 84c80f0

Browse files
authored
feat(semver): Add fields to Release to support semver filtering and sorting. (#25956)
This pr makes changes to the Release model so that we can support semver sorting. The values for these fields will all be extracted from version if it fits our slightly more permissive semver format: <major>.<minor>.<patch>.<revision>-<prerelease>+<build_code> These are all optional to varying extents. If a version does not match this format at all then these fields will just be null. If we partially match, any fields not already included in the version should be set to 0 or '' depending on field type. So 1.2 would have fields like major=1, minor=2, patch=0, revision=0, prerelease=''. This is important, because we want to use https://www.postgresql.org/docs/current/functions-comparisons.html#ROW-WISE-COMPARISON to filter these rows, and nulls cause the comparison to return False where we wouldn't expect. If build_code is entirely numeric we will parse it and store as build_number, which will allow us to sort Android releases numerically. I've added an index on (major, minor, patch, revision) with all the columns desc. We'll likely mostly be sorting desc, so that seemed most helpful. This means that we'd sort in this order: 1.2.1, 1.2.0, with 1.2.1 being the highest. I've left prerelease out of the index, since I think that the number of prereleases per patch or revision will be low, and it's probably nicer to keep the text field out of the index. To filter correctly via semver, we need to add case when prerelease = '' then 1 else 0 end, prerelease to our row comparison, so that rows without a prerelease end up being ranked as higher. This shouldn't be slow, since most rows will have very few prereleases, but if we find that it is we can include these fields in the index.
1 parent 443a1de commit 84c80f0

File tree

3 files changed

+122
-1
lines changed

3 files changed

+122
-1
lines changed

migrations_lockfile.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ will then be regenerated, and you should be able to merge without conflicts.
77

88
jira_ac: 0001_initial
99
nodestore: 0002_nodestore_no_dictfield
10-
sentry: 0198_add_project_transaction_threshold
10+
sentry: 0199_release_semver
1111
social_auth: 0001_initial
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# Generated by Django 1.11.29 on 2021-05-20 20:27
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
# This flag is used to mark that a migration shouldn't be automatically run in
8+
# production. We set this to True for operations that we think are risky and want
9+
# someone from ops to run manually and monitor.
10+
# General advice is that if in doubt, mark your migration as `is_dangerous`.
11+
# Some things you should always mark as dangerous:
12+
# - Large data migrations. Typically we want these to be run manually by ops so that
13+
# they can be monitored. Since data migrations will now hold a transaction open
14+
# this is even more important.
15+
# - Adding columns to highly active tables, even ones that are NULL.
16+
is_dangerous = True
17+
18+
# This flag is used to decide whether to run this migration in a transaction or not.
19+
# By default we prefer to run in a transaction, but for migrations where you want
20+
# to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
21+
# want to create an index concurrently when adding one to an existing table.
22+
# You'll also usually want to set this to `False` if you're writing a data
23+
# migration, since we don't want the entire migration to run in one long-running
24+
# transaction.
25+
atomic = False
26+
27+
dependencies = [
28+
("sentry", "0198_add_project_transaction_threshold"),
29+
]
30+
31+
operations = [
32+
migrations.AddField(
33+
model_name="release",
34+
name="build_code",
35+
field=models.TextField(null=True),
36+
),
37+
migrations.AddField(
38+
model_name="release",
39+
name="build_number",
40+
field=models.BigIntegerField(null=True),
41+
),
42+
migrations.AddField(
43+
model_name="release",
44+
name="major",
45+
field=models.BigIntegerField(null=True),
46+
),
47+
migrations.AddField(
48+
model_name="release",
49+
name="minor",
50+
field=models.BigIntegerField(null=True),
51+
),
52+
migrations.AddField(
53+
model_name="release",
54+
name="patch",
55+
field=models.BigIntegerField(null=True),
56+
),
57+
migrations.AddField(
58+
model_name="release",
59+
name="prerelease",
60+
field=models.TextField(null=True),
61+
),
62+
migrations.AddField(
63+
model_name="release",
64+
name="revision",
65+
field=models.BigIntegerField(null=True),
66+
),
67+
migrations.SeparateDatabaseAndState(
68+
database_operations=[
69+
migrations.RunSQL(
70+
"""
71+
CREATE INDEX CONCURRENTLY IF NOT EXISTS "sentry_release_organization_id_major_mi_38715957_idx"
72+
ON "sentry_release" ("organization_id", "major" DESC, "minor" DESC, "patch" DESC, "revision" DESC);
73+
""",
74+
reverse_sql="DROP INDEX CONCURRENTLY IF EXISTS sentry_release_organization_id_major_mi_38715957_idx",
75+
),
76+
migrations.RunSQL(
77+
"""
78+
CREATE INDEX CONCURRENTLY IF NOT EXISTS "sentry_release_organization_id_build_code_f93815e5_idx" ON "sentry_release" ("organization_id", "build_code");
79+
""",
80+
reverse_sql="DROP INDEX CONCURRENTLY IF EXISTS sentry_release_organization_id_build_code_f93815e5_idx",
81+
),
82+
migrations.RunSQL(
83+
"""
84+
CREATE INDEX CONCURRENTLY IF NOT EXISTS "sentry_release_organization_id_build_number_e1646551_idx" ON "sentry_release" ("organization_id", "build_number");
85+
""",
86+
reverse_sql="DROP INDEX CONCURRENTLY IF EXISTS sentry_release_organization_id_build_number_e1646551_idx",
87+
),
88+
],
89+
state_operations=[
90+
migrations.AlterIndexTogether(
91+
name="release",
92+
index_together={
93+
("organization", "build_code"),
94+
("organization", "major", "minor", "patch", "revision"),
95+
("organization", "build_number"),
96+
},
97+
),
98+
],
99+
),
100+
]

src/sentry/models/release.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,19 @@ class Release(Model):
141141
total_deploys = BoundedPositiveIntegerField(null=True, default=0)
142142
last_deploy_id = BoundedPositiveIntegerField(null=True)
143143

144+
# Denormalized semver columns. These will be filled if `version` matches at least
145+
# part of our more permissive model of semver:
146+
# `<major>.<minor>.<patch>.<revision>-<prerelease>+<build_code>
147+
major = models.BigIntegerField(null=True)
148+
minor = models.BigIntegerField(null=True)
149+
patch = models.BigIntegerField(null=True)
150+
revision = models.BigIntegerField(null=True)
151+
prerelease = models.TextField(null=True)
152+
build_code = models.TextField(null=True)
153+
# If `build_code` can be parsed as a 64 bit int we'll store it here as well for
154+
# sorting/comparison purposes
155+
build_number = models.BigIntegerField(null=True)
156+
144157
# HACK HACK HACK
145158
# As a transitionary step we permit release rows to exist multiple times
146159
# where they are "specialized" for a specific project. The goal is to
@@ -152,6 +165,14 @@ class Meta:
152165
app_label = "sentry"
153166
db_table = "sentry_release"
154167
unique_together = (("organization", "version"),)
168+
# TODO(django2.2): Note that we create this index with each column ordered
169+
# descending. Django 2.2 allows us to specify functional indexes, which should
170+
# allow us to specify this on the model.
171+
index_together = (
172+
("organization", "major", "minor", "patch", "revision"),
173+
("organization", "build_code"),
174+
("organization", "build_number"),
175+
)
155176

156177
__repr__ = sane_repr("organization_id", "version")
157178

0 commit comments

Comments
 (0)