From d02f82086833a14570eced62155120ca725c96f9 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Fri, 25 Sep 2026 12:21:42 +0200 Subject: [PATCH 1/3] Reformat SQL queries --- glean-core/src/database/sqlite.rs | 188 ++++++++++++----------- glean-core/src/database/sqlite/schema.rs | 64 ++++---- 2 files changed, 127 insertions(+), 125 deletions(-) diff --git a/glean-core/src/database/sqlite.rs b/glean-core/src/database/sqlite.rs index 9f43e7840f..859244196a 100644 --- a/glean-core/src/database/sqlite.rs +++ b/glean-core/src/database/sqlite.rs @@ -433,7 +433,9 @@ impl Database { "INSERT INTO lifetime_ping.telemetry SELECT * FROM telemetry WHERE lifetime = 'ping'"; let res = self.conn.write(|tx| tx.execute_one(copy_sql)); if let Err(err) = res { - log::error!("Could not load ping lifetime data into memory: {err:?}. Disabling ping lifetime IO delay."); + log::error!( + "Could not load ping lifetime data into memory: {err:?}. Disabling ping lifetime IO delay." + ); self.delay_ping_lifetime_io = false; } } @@ -466,16 +468,16 @@ impl Database { let table = self.table_for_lifetime(lifetime); let iter_sql = format!( - r#" - SELECT - id, - value, - labels - FROM {table} - WHERE - lifetime = ?1 - AND ping = ?2 - "# + " + SELECT + id, + value, + labels + FROM {table} + WHERE + lifetime = ?1 + AND ping = ?2 + " ); self.conn.read(|conn| { @@ -514,16 +516,16 @@ impl Database { // TODO(bug 2048194): Remove the `LIMIT 1` and error out when more than 1 row is returned. let get_metric_sql = format!( - r#" - SELECT - value - FROM {table} - WHERE - id = ?1 - AND ping = ?2 - AND labels = ?3 - LIMIT 1 - "# + " + SELECT + value + FROM {table} + WHERE + id = ?1 + AND ping = ?2 + AND labels = ?3 + LIMIT 1 + " ); let metric_identifier = &data.base_identifier(); @@ -566,14 +568,14 @@ impl Database { let table = self.table_for_lifetime(lifetime); let has_metric_sql = format!( - r#" - SELECT id - FROM {table} - WHERE - lifetime = ?1 - AND ping = ?2 - AND id = ?3 - "# + " + SELECT id + FROM {table} + WHERE + lifetime = ?1 + AND ping = ?2 + AND id = ?3 + " ); self.conn @@ -647,15 +649,15 @@ impl Database { let table = self.table_for_lifetime(lifetime); let insert_sql = format!( - r#" - INSERT INTO - {table} (id, ping, lifetime, labels, value) - VALUES - (?1, ?2, ?3, ?4, ?5) - ON CONFLICT(id, ping, labels) DO UPDATE SET - lifetime = excluded.lifetime, - value = excluded.value - "# + " + INSERT INTO + {table} (id, ping, lifetime, labels, value) + VALUES + (?1, ?2, ?3, ?4, ?5) + ON CONFLICT(id, ping, labels) DO UPDATE SET + lifetime = excluded.lifetime, + value = excluded.value + " ); { @@ -758,16 +760,16 @@ impl Database { // TODO(bug 2048194): Remove the `LIMIT 1` and error out when more than 1 row is returned. let value_sql = format!( - r#" - SELECT value - FROM {table} - WHERE - id = ?1 - AND ping = ?2 - AND lifetime = ?3 - AND labels = ?4 - LIMIT 1 - "# + " + SELECT value + FROM {table} + WHERE + id = ?1 + AND ping = ?2 + AND lifetime = ?3 + AND labels = ?4 + LIMIT 1 + " ); let new_value = { @@ -789,15 +791,15 @@ impl Database { }; let insert_sql = format!( - r#" - INSERT INTO - {table} (id, ping, lifetime, labels, value) - VALUES - (?1, ?2, ?3, ?4, ?5) - ON CONFLICT(id, ping, labels) DO UPDATE SET - lifetime = excluded.lifetime, - value = excluded.value - "# + " + INSERT INTO + {table} (id, ping, lifetime, labels, value) + VALUES + (?1, ?2, ?3, ?4, ?5) + ON CONFLICT(id, ping, labels) DO UPDATE SET + lifetime = excluded.lifetime, + value = excluded.value + " ); { @@ -1058,17 +1060,17 @@ impl Database { impl StoredSubmittedPingHandler for Database { fn get_all_submitted_pings(&self) -> Vec { - let get_all_submitted_pings_sql = r#" - SELECT - document_id, - ping, - date_submitted, - date_uploaded, - upload_failed, - payload - FROM submitted_pings - ORDER BY date_submitted DESC - "#; + let get_all_submitted_pings_sql = " + SELECT + document_id, + ping, + date_submitted, + date_uploaded, + upload_failed, + payload + FROM submitted_pings + ORDER BY date_submitted DESC + "; self.conn .read(|conn| { let Ok(mut stmt) = conn.prepare_cached(get_all_submitted_pings_sql) else { @@ -1096,19 +1098,19 @@ impl StoredSubmittedPingHandler for Database { } fn get_submitted_pings_by_name(&self, ping: &str) -> Vec { - let get_submitted_pings_sql = r#" - SELECT - document_id, - ping, - date_submitted, - date_uploaded, - upload_failed, - payload - FROM submitted_pings - WHERE - ping = ?1 - ORDER BY date_submitted DESC - "#; + let get_submitted_pings_sql = " + SELECT + document_id, + ping, + date_submitted, + date_uploaded, + upload_failed, + payload + FROM submitted_pings + WHERE + ping = ?1 + ORDER BY date_submitted DESC + "; self.conn .read(|conn| { let Ok(mut stmt) = conn.prepare_cached(get_submitted_pings_sql) else { @@ -1171,18 +1173,18 @@ impl StoredSubmittedPingHandler for Database { payload: JsonValue, ) -> Result<()> { self.conn.write(|tx| { - let insert_sql = r#" - INSERT INTO - submitted_pings (document_id, ping, date_submitted, date_uploaded, upload_failed, payload) - VALUES - (?1, ?2, ?3, ?4, ?5, ?6) - ON CONFLICT(document_id) DO UPDATE SET - ping = excluded.ping, - date_submitted = excluded.date_submitted, - date_uploaded = excluded.date_uploaded, - upload_failed = excluded.upload_failed, - payload = excluded.payload - "#; + let insert_sql = " + INSERT INTO + submitted_pings (document_id, ping, date_submitted, date_uploaded, upload_failed, payload) + VALUES + (?1, ?2, ?3, ?4, ?5, ?6) + ON CONFLICT(document_id) DO UPDATE SET + ping = excluded.ping, + date_submitted = excluded.date_submitted, + date_uploaded = excluded.date_uploaded, + upload_failed = excluded.upload_failed, + payload = excluded.payload + "; let mut stmt = tx.prepare_cached(insert_sql)?; stmt.execute(params![ document_id, diff --git a/glean-core/src/database/sqlite/schema.rs b/glean-core/src/database/sqlite/schema.rs index 28c3f0a36e..2b8e546094 100644 --- a/glean-core/src/database/sqlite/schema.rs +++ b/glean-core/src/database/sqlite/schema.rs @@ -21,14 +21,14 @@ fn table_schema(schema: Option<&str>) -> String { let table_name = "telemetry"; format!( " - CREATE TABLE {schema}{separator}{table_name}( - id TEXT NOT NULL, - ping TEXT NOT NULL, - lifetime TEXT NOT NULL, - labels TEXT NOT NULL, -- can't be null or ON CONFLICT won't work - value BLOB, - UNIQUE(id, ping, labels) - ); + CREATE TABLE {schema}{separator}{table_name}( + id TEXT NOT NULL, + ping TEXT NOT NULL, + lifetime TEXT NOT NULL, + labels TEXT NOT NULL, -- can't be null or ON CONFLICT won't work + value BLOB, + UNIQUE(id, ping, labels) + ); " ) } @@ -41,19 +41,19 @@ impl ConnectionOpener for Schema { fn setup(conn: &mut rusqlite::Connection) -> Result<(), Self::Error> { conn.execute_batch( " - -- we unconditionally want write-ahead-logging mode - PRAGMA journal_mode = WAL; - -- Sync at the most criticial moments, but not with every write - PRAGMA synchronous = NORMAL; - -- limit size of the journal. TODO(bug 2049290): value currently arbitrary. - -- needs refinement. - PRAGMA journal_size_limit = 512000; -- 512 KB. - -- We don't care about temp tables being persisted to disk - PRAGMA temp_store = MEMORY; - -- allows adding incremental cleanup later - PRAGMA auto_vacuum = INCREMENTAL; - -- How long to wait for a lock before returning SQLITE_BUSY (in ms) - PRAGMA busy_timeout = 5000; + -- we unconditionally want write-ahead-logging mode + PRAGMA journal_mode = WAL; + -- Sync at the most criticial moments, but not with every write + PRAGMA synchronous = NORMAL; + -- limit size of the journal. TODO(bug 2049290): value currently arbitrary. + -- needs refinement. + PRAGMA journal_size_limit = 512000; -- 512 KB. + -- We don't care about temp tables being persisted to disk + PRAGMA temp_store = MEMORY; + -- allows adding incremental cleanup later + PRAGMA auto_vacuum = INCREMENTAL; + -- How long to wait for a lock before returning SQLITE_BUSY (in ms) + PRAGMA busy_timeout = 5000; ", )?; @@ -71,17 +71,17 @@ impl ConnectionOpener for Schema { fn create(tx: &mut Transaction<'_>) -> Result<(), Self::Error> { tx.execute_batch(&format!( " - {} - CREATE TABLE migration(id INTEGER PRIMARY KEY, state TEXT NOT NULL); - CREATE TABLE submitted_pings( - document_id TEXT PRIMARY KEY, - ping TEXT NOT NULL, - date_submitted INTEGER NOT NULL, - date_uploaded INTEGER, - upload_failed INTEGER, - payload BLOB - ); - CREATE INDEX submitted_pings_ping on submitted_pings(ping); + {} + CREATE TABLE migration(id INTEGER PRIMARY KEY, state TEXT NOT NULL); + CREATE TABLE submitted_pings( + document_id TEXT PRIMARY KEY, + ping TEXT NOT NULL, + date_submitted INTEGER NOT NULL, + date_uploaded INTEGER, + upload_failed INTEGER, + payload BLOB + ); + CREATE INDEX submitted_pings_ping on submitted_pings(ping); ", table_schema(None) ))?; From fc68aa13e1fac6f157ae71152b322e13b1f0f017 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Wed, 7 Oct 2026 12:29:48 +0200 Subject: [PATCH 2/3] Docs: Add a style guide, detailing what strings in code should look like --- docs/dev/SUMMARY.md | 1 + docs/dev/core/style-guide.md | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 docs/dev/core/style-guide.md diff --git a/docs/dev/SUMMARY.md b/docs/dev/SUMMARY.md index b359716905..747e754914 100644 --- a/docs/dev/SUMMARY.md +++ b/docs/dev/SUMMARY.md @@ -22,6 +22,7 @@ - [Python bindings](python/index.md) - [Setup Build Environment](python/setting-up-python-build-environment.md) - [Rust Component](core/index.md) + - [Style Guide](core/style-guide.md) - [Documentation guidelines](core/documentation-guidelines.md) - [Dependency Management](core/dependency-management.md) - [Dependency Vetting](core/dependency-vetting.md) diff --git a/docs/dev/core/style-guide.md b/docs/dev/core/style-guide.md new file mode 100644 index 0000000000..68bfb55cfc --- /dev/null +++ b/docs/dev/core/style-guide.md @@ -0,0 +1,64 @@ +# Style Guide + +## Code + +All Rust code is formatted using [`rustfmt`](https://github.com/rust-lang/rustfmt). +Run `make fmt-rust` to format code in your local checkout. +This is enforced in CI. + +### Strings in code + +Multi-line strings in Rust code should use double-quotes where possible, or raw string markers (`r#" "#`) if needed. + +When indentation doesn't matter, the double-quote should be on its own line and the start of the string indented by 4 spaces below. +The closing double-quote should be aligned with the identifier. +Unfortunately `rustfmt` will not enforce the intended formatting. + +**Good**: + +```rust +let query = " + SELECT * + FROM table + WHERE id IS NOT NULL +"; +``` + + +**Bad**: + +```rust +let query = "SELECT +* FROM table + WHERE id IS NOT NULL + "; +``` + +When the multi-line string is within a macro (e.g. `format!`), the opening and closing double-quote go on their own line, indented by 4 spaces below the identifier. +`rustfmt` will enforce the quote on its own line, but not the indentation. + +**Good**: + +```rust +let query = format!( + " + SELECT * + FROM {table} + WHERE id IS NOT NULL + " +); +``` + + +**Bad**: + +```rust +let query = format!("SELECT +* FROM {table} + WHERE id IS NOT NULL + "); +``` + +## Documentation + +See [Documentation Guidelines](documentation-guidelines.md) for details. From 007a816546c24f6f72c96b4db37f527c408c69e9 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Wed, 7 Oct 2026 15:19:23 +0200 Subject: [PATCH 3/3] Update docs/dev/core/style-guide.md Co-authored-by: Chris H-C --- docs/dev/core/style-guide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dev/core/style-guide.md b/docs/dev/core/style-guide.md index 68bfb55cfc..781b23b862 100644 --- a/docs/dev/core/style-guide.md +++ b/docs/dev/core/style-guide.md @@ -11,7 +11,7 @@ This is enforced in CI. Multi-line strings in Rust code should use double-quotes where possible, or raw string markers (`r#" "#`) if needed. When indentation doesn't matter, the double-quote should be on its own line and the start of the string indented by 4 spaces below. -The closing double-quote should be aligned with the identifier. +The closing double-quote should be aligned with the beginning of the line that opened the string. Unfortunately `rustfmt` will not enforce the intended formatting. **Good**: