Sqlx Code Review

sqlx Code Review

Review Workflow

1. Check Cargo.toml — Note sqlx features (runtime-tokio, tls-rustls/tls-native-tls, postgres/mysql/sqlite, uuid, chrono, json, migrate) and Rust edition (2024 changes RPIT lifetime capture and removes need for async-trait) 2. Check query patterns — Compile-time checked (query!, query_as!) vs runtime (query, query_as) 3. Check pool configuration — Connection limits, timeouts, idle settings 4. Check migrations — File naming, reversibility, data migration safety 5. Check type mappings — Rust types align with SQL column types

Gates (evidence before severity)

Complete in order; do not assign Critical / Major until the gate for that claim is passed.

1. Scope — Identify the crate under review (Cargo.toml path) and the .rs files (or directory) you opened. Pass: At least one concrete path you inspected is named. 2. sqlx / compile claims — Before asserting issues about query! / query_as!, offline mode, sqlx.toml, DATABASE_URL, or Cargo features: open the relevant Cargo.toml and, if applicable, sqlx.toml or documented env. Pass: The finding cites a line or you state that those files were absent / out of scope. 3. Finding anchors — Each reported issue includes [FILE:LINE] per Output Format. Pass: No Critical or Major without a line reference. 4. Protocol — Load and complete the review-verification-protocol skill after gates 1–3 and before final severity labels. Pass: Protocol steps satisfied for each retained finding.

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

Review Checklist

Query Patterns

• [ ] Compile-time checked queries (query!, query_as!) used where possible
• [ ] sqlx.toml or DATABASE_URL configured for offline compile-time checking
• [ ] No string interpolation in queries (SQL injection risk) — use bind parameters ($1, $2)
• [ ] query_as! maps to named structs, not anonymous records, for public APIs
• [ ] .fetch_one(), .fetch_optional(), .fetch_all() chosen appropriately
• [ ] .fetch() (streaming) used for large result sets

Connection Pool

• [ ] PgPool shared via Arc or framework state (not created per-request)
• [ ] Pool size configured for the deployment (not left at defaults in production)
• [ ] Connection acquisition timeout set
• [ ] Idle connection cleanup configured
• [ ] Edition 2024: Pool initialization uses std::sync::LazyLock (not once_cell::sync::Lazy or lazy_static!) for static pool singletons

Transactions

• [ ] pool.begin() used for multi-statement operations
• [ ] Transaction committed explicitly (not relying on implicit rollback on drop)
• [ ] Errors within transactions trigger rollback before propagation
• [ ] Nested transactions use savepoints (tx.begin()) if needed

Type Mapping

• [ ] sqlx::Type derives match database column types
• [ ] Enum representations consistent between Rust, serde, and SQL
• [ ] Uuid, DateTime<Utc>, Decimal types used (not strings for structured data)
• [ ] Option<T> used for nullable columns
• [ ] serde_json::Value used for JSONB columns
• [ ] No enum variants or struct fields named gen — reserved keyword in edition 2024 (use r#gen with #[sqlx(rename = "gen")] or choose a different name)

Edition 2024 Compatibility

• [ ] Functions returning -> impl Stream or -> impl Future account for RPIT lifetime capture changes (all in-scope lifetimes captured by default; use + use<'a> for precise control)
• [ ] Custom FromRow or Type trait impls use native async fn in traits where applicable (no #[async_trait] needed, stable since Rust 1.75)
• [ ] Prefer #[expect(unused)] over #[allow(unused)] for compile-time query fields only used in some code paths (self-cleaning lint suppression, stable since 1.81)
• [ ] Static pool initialization uses std::sync::LazyLock (not once_cell or lazy_static!)

Migrations

• [ ] Migration files follow naming convention (YYYYMMDDHHMMSS_description.sql)
• [ ] Destructive migrations (DROP, ALTER DROP COLUMN) are reversible or have data backup plan
• [ ] No data-dependent schema changes in same migration as data changes
• [ ] sqlx::migrate!() called at application startup

Severity Calibration

Critical

• String interpolation in SQL queries (SQL injection)
• Missing transaction for multi-statement writes (partial writes on error)
• Connection pool created per-request (connection exhaustion)
• Missing bind parameter escaping

Major

• Runtime queries (query()) where compile-time (query!()) could verify correctness
• Missing transaction rollback on error paths
• Enum type mismatch between Rust and database
• Unbounded .fetch_all() on potentially large tables
• Field or variant named gen without r#gen escape (edition 2024 compile failure)

Minor

• Pool defaults used in production without tuning
• Missing .fetch_optional() (using .fetch_one() then handling error for "not found")
• Overly broad SELECT * when only specific columns needed
• Missing indexes for queried columns (flag only if query pattern is clearly slow)
• Edition 2024: once_cell::sync::Lazy or lazy_static! used where std::sync::LazyLock works
• Using #[allow(unused)] instead of #[expect(unused)] for query fields (prefer self-cleaning lint suppression)

Informational

• Suggestions to use query_as! for type-safe result mapping
• Suggestions to add database-level constraints alongside Rust validation
• Migration organization improvements

Valid Patterns (Do NOT Flag)

• Runtime `query()` for dynamic queries — Compile-time checking doesn't work with dynamic SQL
• `sqlx::FromRow` derive — Valid alternative to query_as! for reusable row types
• `TEXT` columns for enum storage — Valid with sqlx::Type derive, simpler than custom SQL types
• `.execute()` ignoring row count — Acceptable for idempotent operations (upserts, deletes)
• Shared DB with other languages — e.g., Elixir owns migrations, Rust reads. This is a valid architecture.
• `r#gen` with `#[sqlx(rename = "gen")]` — Correct edition 2024 workaround for gen columns in database types
• `+ use<'a>` on query helper return types — Precise RPIT lifetime capture (edition 2024)
• `std::sync::LazyLock` for static pool initialization — Replaces once_cell/lazy_static (stable since Rust 1.80)
• Native `async fn` in custom `FromRow`/`Type` trait impls — async-trait crate no longer needed (stable since Rust 1.75)

Before Submitting Findings

Complete Gates (evidence before severity), then load and follow the review-verification-protocol skill before reporting any issue.

Migrations and Pool Management

Connection Pool Configuration

Creating the Pool

use sqlx::postgres::PgPoolOptions;

let pool = PgPoolOptions::new()
    .max_connections(20)
    .min_connections(5)
    .acquire_timeout(Duration::from_secs(5))
    .idle_timeout(Duration::from_secs(600))
    .max_lifetime(Duration::from_secs(1800))
    .connect(&database_url)
    .await?;

Edition 2024: Static Pool with LazyLock

For applications that initialize a global pool once, use std::sync::LazyLock instead of once_cell::sync::Lazy or lazy_static!. LazyLock is in std since Rust 1.80.

// BAD — third-party crate, unnecessary dependency
use once_cell::sync::Lazy;

static POOL: Lazy<PgPool> = Lazy::new(|| {
    tokio::runtime::Handle::current().block_on(async {
        PgPoolOptions::new()
            .max_connections(20)
            .connect(&std::env::var("DATABASE_URL").unwrap())
            .await
            .unwrap()
    })
});

// GOOD — std library, no extra dependency
use std::sync::LazyLock;

static POOL: LazyLock<PgPool> = LazyLock::new(|| {
    tokio::runtime::Handle::current().block_on(async {
        PgPoolOptions::new()
            .max_connections(20)
            .connect(&std::env::var("DATABASE_URL").unwrap())
            .await
            .unwrap()
    })
});

Note: Framework-managed state (e.g., axum State<PgPool>) is still preferred over global statics. Use LazyLock only when a static singleton is genuinely needed.

Pool Sizing Guidelines

• Web servers: 2-4× the number of async worker threads
• Background workers: Match to the number of concurrent jobs
• Default (5): Too low for most production workloads
• Maximum: Don't exceed the database's max_connections minus connections reserved for admin/monitoring

Common Mistake: Pool Per Request

// BAD - creates a new pool (and connections) per request
async fn handle(req: Request) -> Response {
    let pool = PgPool::connect(&url).await.unwrap();
    let user = query_as!(User, "...", id).fetch_one(&pool).await?;
    // pool dropped, connections closed
}

// GOOD - share pool via application state
async fn handle(State(pool): State<PgPool>, req: Request) -> Response {
    let user = query_as!(User, "...", id).fetch_one(&pool).await?;
}

Transactions

Basic Pattern

let mut tx = pool.begin().await?;

sqlx::query!("INSERT INTO orders (user_id, total) VALUES ($1, $2)", user_id, total)
    .execute(&mut *tx)
    .await?;

sqlx::query!("UPDATE inventory SET count = count - $1 WHERE item_id = $2", qty, item_id)
    .execute(&mut *tx)
    .await?;

tx.commit().await?;

If tx is dropped without calling .commit(), the transaction rolls back automatically. This is a safety net — explicit commit is clearer.

Error Handling in Transactions

async fn create_order(pool: &PgPool, order: NewOrder) -> Result<Order, Error> {
    let mut tx = pool.begin().await?;

    let order = sqlx::query_as!(Order, "INSERT INTO orders ... RETURNING *", ...)
        .fetch_one(&mut *tx)
        .await
        .map_err(|e| {
            // tx will rollback on drop
            Error::OrderCreate { source: e }
        })?;

    for item in &order.items {
        sqlx::query!("INSERT INTO order_items ...", ...)
            .execute(&mut *tx)
            .await?;
    }

    tx.commit().await?;
    Ok(order)
}

Savepoints (Nested Transactions)

let mut tx = pool.begin().await?;

// Outer operation
sqlx::query!("INSERT INTO audit_log ...").execute(&mut *tx).await?;

// Inner operation that might fail independently
let savepoint = tx.begin().await?; // creates a savepoint
match try_optional_operation(&mut *savepoint).await {
    Ok(_) => savepoint.commit().await?,
    Err(_) => {
        // savepoint rolls back, outer transaction continues
        tracing::warn!("optional operation failed, continuing");
    }
}

tx.commit().await?;

Migrations

File Structure

migrations/
├── 20240115100000_create_users.sql
├── 20240115100001_create_orders.sql
└── 20240220150000_add_user_email.sql

Running Migrations

// At application startup
sqlx::migrate!("./migrations")
    .run(&pool)
    .await?;

Migration Safety Rules

1. Never modify an applied migration — Create a new one instead 2. Separate schema and data migrations — Mixing DDL and DML in one migration increases lock contention and makes rollbacks operationally risky 3. Make migrations idempotent where possible — IF NOT EXISTS, IF EXISTS 4. Destructive migrations need a plan — DROP COLUMN and DROP TABLE should be preceded by a data migration that archives/moves data

Reversible Migration Pattern

-- 20240220150000_add_user_email.sql
ALTER TABLE users ADD COLUMN IF NOT EXISTS email TEXT;

-- To reverse (keep as documentation or in a separate down file):
-- ALTER TABLE users DROP COLUMN IF EXISTS email;

Review Questions

1. Is the pool shared across the application (not created per-request)? 2. Is pool size configured appropriately for the deployment? 3. Are transactions used for multi-statement writes? 4. Are transactions committed explicitly? 5. Are migrations safe (no data loss, idempotent, separate DDL/DML)? 6. Is sqlx::migrate!() called at startup? 7. (Edition 2024) Does static pool initialization use std::sync::LazyLock instead of once_cell or lazy_static!?

Queries

Compile-Time Checked Queries

sqlx verifies queries against the database schema at compile time. This catches column name typos, type mismatches, and invalid SQL before runtime.

query! Macro

Returns an anonymous struct with fields matching the query columns.

// Compile-time checked — column names and types verified
let row = sqlx::query!(
    "SELECT id, name, email FROM users WHERE id = $1",
    user_id
)
.fetch_one(&pool)
.await?;

let name: String = row.name;
let email: Option<String> = row.email; // nullable column → Option

query_as! Macro

Maps results directly to a named struct. Preferred for reusable result types.

#[derive(Debug)]
struct User {
    id: Uuid,
    name: String,
    email: Option<String>,
    created_at: DateTime<Utc>,
}

let user = sqlx::query_as!(
    User,
    "SELECT id, name, email, created_at FROM users WHERE id = $1",
    user_id
)
.fetch_optional(&pool)
.await?;

Offline Mode

For CI/CD where the database isn't available, sqlx caches query metadata. In sqlx 0.8+, configuration lives in sqlx.toml and cached metadata is stored in the .sqlx/ directory. Older versions used sqlx-data.json.

# Generate cache from live database
cargo sqlx prepare

# Build uses cached metadata when DATABASE_URL is absent
cargo build

In sqlx 0.8+, you can configure offline mode and other settings via sqlx.toml:

# sqlx.toml
[common]
offline = true
column-override = {}

Fetch Methods

Common Mistake: fetch_one for Lookups

// BAD - returns Err(RowNotFound) on "not found" which is an expected case
let user = sqlx::query_as!(User, "SELECT ... WHERE id = $1", id)
    .fetch_one(&pool)
    .await?; // RowNotFound error for missing users

// GOOD - "not found" is a normal case, not an error
let user = sqlx::query_as!(User, "SELECT ... WHERE id = $1", id)
    .fetch_optional(&pool)
    .await?;
match user {
    Some(user) => Ok(user),
    None => Err(Error::NotFound(id)),
}

Streaming Large Results

use futures::TryStreamExt;

let mut stream = sqlx::query_as!(Event, "SELECT * FROM events WHERE workflow_id = $1", wf_id)
    .fetch(&pool);

while let Some(event) = stream.try_next().await? {
    process(event).await;
}

Edition 2024: RPIT Lifetime Capture in Query Helpers

In edition 2024, -> impl Trait captures all in-scope lifetimes by default. This affects functions that return streams or futures from sqlx queries.

// Edition 2021 — worked because `-> impl Stream` didn't capture 'a
fn get_events<'a>(pool: &'a PgPool, wf_id: Uuid) -> impl Stream<Item = Result<Event, sqlx::Error>> {
    sqlx::query_as!(Event, "SELECT * FROM events WHERE workflow_id = $1", wf_id)
        .fetch(pool)
}

// Edition 2024 — captures 'a by default, which is usually correct here.
// If you need to NOT capture a lifetime, use precise capture syntax:
fn get_events<'a>(pool: &'a PgPool, wf_id: Uuid) -> impl Stream<Item = Result<Event, sqlx::Error>> + use<'a> {
    sqlx::query_as!(Event, "SELECT * FROM events WHERE workflow_id = $1", wf_id)
        .fetch(pool)
}

Most sqlx query helpers that borrow the pool should capture the pool lifetime, so the edition 2024 default is usually correct. Flag cases where the return type is stored in a struct that outlives the borrow.

Bind Parameters

Always use bind parameters ($1, $2 for Postgres; ? for MySQL/SQLite). Never interpolate values into query strings.

// BAD - SQL injection vulnerability
let query = format!("SELECT * FROM users WHERE name = '{}'", name);
sqlx::query(&query).fetch_one(&pool).await?;

// GOOD - parameterized query
sqlx::query("SELECT * FROM users WHERE name = $1")
    .bind(&name)
    .fetch_one(&pool)
    .await?;

// BEST - compile-time checked
sqlx::query!("SELECT * FROM users WHERE name = $1", name)
    .fetch_one(&pool)
    .await?;

Type Mapping

Rust ↔ PostgreSQL

Custom Enum Types

#[derive(Debug, Clone, sqlx::Type, Serialize, Deserialize)]
#[sqlx(type_name = "varchar", rename_all = "snake_case")]
#[serde(rename_all = "snake_case")]
pub enum Status {
    Pending,
    InProgress,
    Complete,
    Failed,
}

Ensure rename_all matches between sqlx::Type and serde — mismatches cause silent bugs where data written by one system can't be read by the other.

Edition 2024: Reserved gen Keyword

In edition 2024, gen is a reserved keyword. Any sqlx enum variant or struct field named gen will fail to compile. Use r#gen as the Rust identifier and #[sqlx(rename)] to preserve the database column name.

// BAD — fails to compile on edition 2024
#[derive(sqlx::Type)]
#[sqlx(type_name = "varchar", rename_all = "snake_case")]
pub enum GenerationType {
    Manual,
    Gen, // compile error: `gen` is a reserved keyword
}

// GOOD — compiles on edition 2024, database value unchanged
#[derive(sqlx::Type)]
#[sqlx(type_name = "varchar", rename_all = "snake_case")]
pub enum GenerationType {
    Manual,
    #[sqlx(rename = "gen")]
    r#Gen,
}

Edition 2024: #[expect] for Lint Suppression

Prefer #[expect(unused)] over #[allow(unused)] for struct fields that exist only for sqlx mapping but aren't read directly. The #[expect] attribute warns when the suppression becomes unnecessary, keeping lint overrides self-cleaning.

// BAD — silent if the field starts being used elsewhere
#[allow(dead_code)]
struct AuditRow {
    id: i64,
    raw_payload: serde_json::Value,
}

// GOOD — warns when suppression is no longer needed
#[expect(dead_code)]
struct AuditRow {
    id: i64,
    raw_payload: serde_json::Value,
}

Review Questions

1. Are queries compile-time checked where possible? 2. Is .fetch_optional() used for lookups that may return no rows? 3. Are bind parameters used (no string interpolation)? 4. Is .fetch() streaming used for large result sets? 5. Do Rust types match PostgreSQL column types? 6. Are enum representations consistent between sqlx and serde? 7. (Edition 2024) Do any enum variants or fields use gen as an identifier without r#gen? 8. (Edition 2024) Do functions returning -> impl Stream/-> impl Future account for RPIT lifetime capture changes?