Skip to main content

max / makenotwork

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