Skip to main content

max / makenotwork

11.0 KB · 317 lines History Blame Raw
1 //! CI guard for migration safety hygiene (forward-only).
2 //!
3 //! sqlx checksums every *applied* migration, so historical migrations can never
4 //! be edited, retrofitting `CONCURRENTLY` / `IF NOT EXISTS` onto migrations
5 //! `001..=HIGH_WATER` is impossible without breaking the checksum guard. This
6 //! test therefore freezes the existing set and enforces the conventions only on
7 //! NEW migrations (number > `HIGH_WATER`):
8 //!
9 //! 1. A `CREATE INDEX` on a high-write "growth" table must be `CONCURRENTLY`.
10 //! A plain `CREATE INDEX` takes an `ACCESS EXCLUSIVE` lock and blocks writes
11 //! for the whole build; on a grown `transactions`/`page_views` table that is
12 //! a production write-stall.
13 //! 2. `CREATE INDEX CONCURRENTLY` cannot run inside a transaction, so any file
14 //! that uses it must opt out of sqlx's per-migration transaction by starting
15 //! with the exact bytes `-- no-transaction` (sqlx-core 0.8 `source.rs:127`
16 //! does `sql.starts_with("-- no-transaction")`, it must be the file prefix,
17 //! no leading blank line).
18 //! 3. `CREATE TABLE` / `CREATE INDEX` must be `IF NOT EXISTS` (re-run safety,
19 //! a partially-applied migration set can be re-run without erroring).
20 //!
21 //! Bump `HIGH_WATER` only after deliberately reviewing every migration at or below
22 //! the new mark and accepting its state.
23 //!
24 //! Run with: cargo test --test migration_hygiene
25
26 use std::fs;
27 use std::path::Path;
28
29 /// Highest migration number whose contents are grandfathered. Landed at the
30 /// current max during the Run 23 remediation (2026-07-04). Anything strictly
31 /// greater must satisfy the conventions above.
32 const HIGH_WATER: u32 = 167;
33
34 /// Tables large/hot enough that a blocking (non-concurrent) index build is a real
35 /// production write-stall. A `CREATE INDEX` touching one of these in a new
36 /// migration must be `CONCURRENTLY`.
37 const GROWTH_TABLES: &[&str] = &[
38 "transactions",
39 "page_views",
40 "follows",
41 "subscriptions",
42 "items",
43 "item_versions",
44 "sync_blobs",
45 "webhook_events",
46 "processed_webhook_events",
47 ];
48
49 const MIGRATIONS_DIR: &str = "migrations";
50
51 #[test]
52 fn new_migrations_follow_index_and_rerun_safety_conventions() {
53 let mut violations = Vec::new();
54
55 let mut files: Vec<_> = fs::read_dir(MIGRATIONS_DIR)
56 .expect("read migrations dir")
57 .filter_map(std::result::Result::ok)
58 .map(|e| e.path())
59 .filter(|p| p.extension().is_some_and(|x| x == "sql"))
60 .collect();
61 files.sort();
62
63 for path in &files {
64 let Some(num) = leading_number(path) else {
65 violations.push(format!("{}: filename has no leading number", show(path)));
66 continue;
67 };
68 if num <= HIGH_WATER {
69 continue; // grandfathered, frozen by sqlx checksum
70 }
71
72 let raw = fs::read_to_string(path).expect("read migration");
73 let no_tx = raw.starts_with("-- no-transaction");
74 let stmts = statements(&raw);
75
76 for stmt in &stmts {
77 if let Some(idx) = parse_create_index(stmt) {
78 if idx.growth_target() && !idx.concurrently {
79 violations.push(format!(
80 "{}: CREATE INDEX on growth table `{}` must be CONCURRENTLY \
81 (plain build takes ACCESS EXCLUSIVE and stalls writes)",
82 show(path),
83 idx.table
84 ));
85 }
86 if idx.concurrently && !no_tx {
87 violations.push(format!(
88 "{}: uses CREATE INDEX CONCURRENTLY but the file does not start \
89 with `-- no-transaction` — sqlx will wrap it in a transaction \
90 and Postgres will reject it at runtime",
91 show(path)
92 ));
93 }
94 if !idx.if_not_exists {
95 violations.push(format!(
96 "{}: CREATE INDEX must be IF NOT EXISTS for re-run safety",
97 show(path)
98 ));
99 }
100 } else if is_create_table(stmt) && !has_if_not_exists(stmt) {
101 violations.push(format!(
102 "{}: CREATE TABLE must be IF NOT EXISTS for re-run safety",
103 show(path)
104 ));
105 }
106 }
107 }
108
109 assert!(
110 violations.is_empty(),
111 "migration hygiene violations (see tests/migration_hygiene.rs for the rules):\n{}",
112 violations.join("\n")
113 );
114 }
115
116 /// The guard must not silently pass because every new migration was skipped by a
117 /// wrong `HIGH_WATER`: assert the mark is not below the real maximum on disk.
118 /// (If someone adds `168_*.sql`, the mark stays at 167 and 168 is checked, good.
119 /// This only fires if `HIGH_WATER` is set *above* the newest file, which would
120 /// disable the guard entirely.)
121 #[test]
122 fn high_water_mark_is_not_ahead_of_disk() {
123 let max = fs::read_dir(MIGRATIONS_DIR)
124 .expect("read migrations dir")
125 .filter_map(std::result::Result::ok)
126 .filter_map(|e| leading_number(&e.path()))
127 .max()
128 .expect("at least one migration");
129 assert!(
130 HIGH_WATER <= max,
131 "HIGH_WATER ({HIGH_WATER}) is ahead of the newest migration ({max}) — \
132 the hygiene guard would skip every file. Lower HIGH_WATER."
133 );
134 }
135
136 // ---- helpers -------------------------------------------------------------
137
138 fn show(p: &Path) -> String {
139 p.file_name().unwrap().to_string_lossy().into_owned()
140 }
141
142 fn leading_number(p: &Path) -> Option<u32> {
143 let name = p.file_name()?.to_str()?;
144 let digits: String = name.chars().take_while(char::is_ascii_digit).collect();
145 digits.parse().ok()
146 }
147
148 /// Split into statements: strip `--` line comments and `/* */` block comments,
149 /// lowercase, collapse whitespace, split on `;`.
150 fn statements(raw: &str) -> Vec<String> {
151 // Strip block comments. Uses `str::find` (whose offsets are always char
152 // boundaries) and `push_str`, so it is UTF-8-safe, an earlier byte-index
153 // version panicked when a migration comment contained a multibyte char
154 // like '...'.
155 let mut no_block = String::with_capacity(raw.len());
156 let mut rest = raw;
157 while let Some(start) = rest.find("/*") {
158 no_block.push_str(&rest[..start]);
159 no_block.push(' ');
160 match rest[start + 2..].find("*/") {
161 Some(end) => rest = &rest[start + 2 + end + 2..],
162 None => {
163 rest = "";
164 break;
165 }
166 }
167 }
168 no_block.push_str(rest);
169 // Strip line comments and normalize.
170 let cleaned: String = no_block
171 .lines()
172 .map(|l| match l.find("--") {
173 Some(pos) => &l[..pos],
174 None => l,
175 })
176 .collect::<Vec<_>>()
177 .join(" ");
178 let norm = cleaned.to_lowercase();
179 norm.split(';')
180 .map(|s| s.split_whitespace().collect::<Vec<_>>().join(" "))
181 .filter(|s| !s.is_empty())
182 .collect()
183 }
184
185 fn is_create_table(stmt: &str) -> bool {
186 stmt.starts_with("create table") || stmt.starts_with("create unlogged table")
187 }
188
189 fn has_if_not_exists(stmt: &str) -> bool {
190 stmt.contains("if not exists")
191 }
192
193 struct IndexStmt {
194 concurrently: bool,
195 if_not_exists: bool,
196 table: String,
197 }
198
199 impl IndexStmt {
200 fn growth_target(&self) -> bool {
201 GROWTH_TABLES.contains(&self.table.as_str())
202 }
203 }
204
205 /// Parse a `CREATE [UNIQUE] INDEX [CONCURRENTLY] [IF NOT EXISTS] name ON table ...`.
206 /// Returns None if the statement is not a create-index.
207 fn parse_create_index(stmt: &str) -> Option<IndexStmt> {
208 let tokens: Vec<&str> = stmt.split_whitespace().collect();
209 // Must begin create [unique] index
210 let idx_pos = tokens.iter().position(|t| *t == "index")?;
211 if tokens.first() != Some(&"create") {
212 return None;
213 }
214 match idx_pos {
215 1 => {} // create index
216 2 if tokens.get(1) == Some(&"unique") => {} // create unique index
217 _ => return None,
218 }
219 let after: Vec<&str> = tokens[idx_pos + 1..].to_vec();
220 let concurrently = after.first() == Some(&"concurrently");
221 let if_not_exists = stmt.contains("if not exists");
222 // Table name is the token following `on`.
223 let on_pos = after.iter().position(|t| *t == "on")?;
224 let raw_table = after.get(on_pos + 1)?;
225 let table = raw_table
226 .trim_matches(|c| c == '"' || c == '(')
227 .split('(')
228 .next()
229 .unwrap_or(raw_table)
230 .to_string();
231 Some(IndexStmt {
232 concurrently,
233 if_not_exists,
234 table,
235 })
236 }
237
238 // ---- self-tests: prove the detector fires, so the on-disk pass is not vacuous ----
239
240 #[cfg(test)]
241 mod detector_tests {
242 use super::*;
243
244 fn only(stmt: &str) -> String {
245 statements(stmt).into_iter().next().unwrap()
246 }
247
248 #[test]
249 fn flags_plain_index_on_growth_table() {
250 let idx = parse_create_index(&only(
251 "CREATE INDEX IF NOT EXISTS idx_x ON transactions (seller_user_id);",
252 ))
253 .unwrap();
254 assert!(idx.growth_target());
255 assert!(!idx.concurrently, "plain build must be caught");
256 assert!(idx.if_not_exists);
257 }
258
259 #[test]
260 fn accepts_concurrent_index_on_growth_table() {
261 let idx = parse_create_index(&only(
262 "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_x ON transactions (created_at);",
263 ))
264 .unwrap();
265 assert!(idx.growth_target());
266 assert!(idx.concurrently);
267 assert!(idx.if_not_exists);
268 }
269
270 #[test]
271 fn small_table_index_is_not_a_growth_target() {
272 let idx =
273 parse_create_index(&only("CREATE INDEX IF NOT EXISTS i ON promo_codes (id);")).unwrap();
274 assert!(!idx.growth_target(), "non-hot table needn't be concurrent");
275 }
276
277 #[test]
278 fn detects_missing_if_not_exists() {
279 let idx = parse_create_index(&only("CREATE INDEX i ON items (slug);")).unwrap();
280 assert!(!idx.if_not_exists);
281 assert!(idx.growth_target());
282 }
283
284 #[test]
285 fn no_transaction_directive_must_be_file_prefix() {
286 // sqlx does sql.starts_with("-- no-transaction"), a leading blank line breaks it.
287 assert!(
288 "-- no-transaction\nCREATE INDEX CONCURRENTLY ...".starts_with("-- no-transaction")
289 );
290 assert!(!"\n-- no-transaction\n...".starts_with("-- no-transaction"));
291 }
292
293 #[test]
294 fn unique_index_parsed_and_table_extracted_without_paren() {
295 let idx = parse_create_index(&only(
296 "CREATE UNIQUE INDEX CONCURRENTLY i ON subscriptions(user_id);",
297 ))
298 .unwrap();
299 assert_eq!(idx.table, "subscriptions");
300 assert!(idx.concurrently);
301 }
302
303 #[test]
304 fn create_table_without_ine_is_flagged() {
305 let s = only("CREATE TABLE foo (id UUID PRIMARY KEY);");
306 assert!(is_create_table(&s));
307 assert!(!has_if_not_exists(&s));
308 }
309
310 #[test]
311 fn line_and_block_comments_are_stripped() {
312 let s = statements("/* c */ CREATE INDEX i ON items (a); -- trailing\n");
313 assert_eq!(s.len(), 1);
314 assert!(parse_create_index(&s[0]).is_some());
315 }
316 }
317