diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..f09ad20 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,50 @@ +{ + "$schema": "https://raw.githubusercontent.com/devcontainers/spec/refs/heads/main/schemas/devContainer.base.schema.json", + "name": "sqlx-query", + "service": "workspace", + "dockerComposeFile": "docker-compose.yml", + "workspaceFolder": "/home/vscode/workspace", + "features": { + "ghcr.io/devcontainers/features/nix:1": { + // Single-user install. The feature defaults to multiUser, which runs + // post-install steps through the nix-daemon — but a Docker build layer + // has no init system to start one, so those steps fail with + // "opening lock file /nix/var/nix/db/big-lock: Permission denied". + // A single-user store owned by vscode needs no daemon, and is what the + // /nix volume and the postStartCommand chown below already assume. + "multiUser": false, + "extraNixConfig": "experimental-features = nix-command flakes" + } + }, + // The single definition of the test database. Inside the container this + // resolves over the compose network; on the host, `devcontainer-env export` + // rewrites it to the port Docker assigned. `containerEnv` rather than + // `remoteEnv` because only the former applies to every process, which is + // what devcontainer-env reads. + "containerEnv": { + "DATABASE_URL": "postgres://vscode@postgres:5432/sqlx_query_test?sslmode=disable" + }, + "mounts": [ + "source=${localWorkspaceFolderBasename}-nix,target=/nix,type=volume", + "source=${localWorkspaceFolderBasename}-cache,target=/home/vscode/.cache,type=volume", + "source=${localWorkspaceFolderBasename}-cargo,target=/home/vscode/.cargo,type=volume" + ], + // Every volume stays *outside* the workspace. Mounting one at + // workspace/target would create that directory inside the bind mount, owned + // by the container's uid -- and CI runs cargo on the runner itself, where + // that is a different uid and `target/` becomes unwritable. + "postStartCommand": "sudo chown vscode:vscode /nix /home/vscode/.cache /home/vscode/.cargo", + "customizations": { + "vscode": { + "settings": { + "terminal.integrated.defaultProfile.linux": "default", + "terminal.integrated.profiles.linux": { + "default": { + "path": "nix", + "args": ["develop"] + } + } + } + } + } +} diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml new file mode 100644 index 0000000..5c37451 --- /dev/null +++ b/.devcontainer/docker-compose.yml @@ -0,0 +1,27 @@ +services: + workspace: + image: mcr.microsoft.com/devcontainers/base:bookworm + command: sleep infinity + volumes: + - ..:/home/vscode/workspace:cached + + # What tests/postgres.rs runs against. `trust` auth is safe only because this + # is never reachable off the machine -- see the port note below. + postgres: + image: postgres:18-bookworm + restart: unless-stopped + environment: + POSTGRES_USER: vscode + POSTGRES_DB: sqlx_query_test + POSTGRES_HOST_AUTH_METHOD: trust + healthcheck: + test: ["CMD-SHELL", "pg_isready"] + interval: 1s + timeout: 5s + retries: 10 + # Bare `5432`, not "5432:5432": Docker assigns a random host port, so two + # projects can run at once without colliding. `devcontainer-env export` + # finds the assigned port and rewrites DATABASE_URL to match, which is what + # lets the flake and CI share one definition of it. + ports: + - 5432 diff --git a/.github/config/release-please-config.json b/.github/config/release-please-config.json new file mode 100644 index 0000000..18d60f0 --- /dev/null +++ b/.github/config/release-please-config.json @@ -0,0 +1,11 @@ +{ + "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json", + "packages": { + ".": { + "release-type": "rust", + "include-v-in-tag": true, + "include-v-in-release-name": true, + "include-component-in-tag": false + } + } +} diff --git a/.github/config/release-please-manifest.json b/.github/config/release-please-manifest.json new file mode 100644 index 0000000..e18ee07 --- /dev/null +++ b/.github/config/release-please-manifest.json @@ -0,0 +1,3 @@ +{ + ".": "0.0.0" +} diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..523d50f --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,30 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + groups: + all-github-actions: + patterns: ["*"] + labels: + - "dependencies" + - "auto-merge" + commit-message: + prefix: "chore" + include: "scope" + - package-ecosystem: "cargo" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + groups: + all-cargo: + patterns: ["*"] + labels: + - "dependencies" + - "auto-merge" + commit-message: + prefix: "chore" + include: "scope" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..21b6f3f --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,99 @@ +name: CI +on: + push: + branches: [main] + pull_request: + branches: [main] +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} +jobs: + test: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v7 + # Brings up .devcontainer/docker-compose.yml -- including the Postgres + # tests/postgres.rs runs against -- and tears it down in a post-step even + # if the job is cancelled. DATABASE_URL is not set here: the shell hook + # gets it from `devcontainer-env export`, so it is defined once, in + # devcontainer.json. + # + # Nix then runs on the runner itself rather than inside the container. + # Building the devcontainer anyway is what keeps its definition honest -- + # a broken one fails CI instead of only failing the next contributor. + - name: Setup Devcontainer + uses: devcontainer-env/devcontainer-ci@v1 + - name: Install Nix + uses: DeterminateSystems/nix-installer-action@v22 + - name: Setup Nix Cache + uses: DeterminateSystems/magic-nix-cache-action@v14 + with: + use-flakehub: disabled + use-gha-cache: enabled + - name: Check Formatting + run: nix develop --command cargo fmt --check + - name: Lint + run: nix develop --command cargo clippy --all-targets + - name: Run Tests + run: nix develop --command cargo test + - name: Check Documentation + run: nix develop --command cargo doc --no-deps + env: + RUSTDOCFLAGS: -D warnings + release: + needs: test + if: github.event_name == 'push' + runs-on: ubuntu-latest + permissions: + contents: write + issues: write + pull-requests: write + timeout-minutes: 15 + outputs: + release_created: ${{ steps.release.outputs.release_created }} + tag_name: ${{ steps.release.outputs.tag_name }} + steps: + - name: Checkout + uses: actions/checkout@v7 + with: + fetch-depth: 0 + - name: Create Release PR + id: release + uses: googleapis/release-please-action@v5 + with: + manifest-file: .github/config/release-please-manifest.json + config-file: .github/config/release-please-config.json + target-branch: main + # No `build` job: this is a library crate with no binary, so there is nothing + # to `nix build` and no asset to attach to the release. The crate itself is + # the artifact, and it goes to crates.io below. + publish: + needs: release + if: ${{ needs.release.outputs.release_created }} + runs-on: ubuntu-latest + permissions: + contents: read + id-token: write + timeout-minutes: 15 + steps: + - name: Checkout + uses: actions/checkout@v7 + - name: Install Nix + uses: DeterminateSystems/nix-installer-action@v22 + - name: Setup Nix Cache + uses: DeterminateSystems/magic-nix-cache-action@v14 + with: + use-flakehub: disabled + use-gha-cache: enabled + - name: Authenticate with Crates.io + uses: rust-lang/crates-io-auth-action@v1 + id: auth + - name: Publish to Crates.io + run: nix develop --command cargo publish + env: + CARGO_REGISTRY_TOKEN: ${{ steps.auth.outputs.token }} diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml new file mode 100644 index 0000000..286790b --- /dev/null +++ b/.github/workflows/merge.yml @@ -0,0 +1,26 @@ +name: Auto Merge + +on: + pull_request: + types: [opened, reopened, labeled, synchronize] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true + +jobs: + merge: + if: contains(github.event.pull_request.labels.*.name, 'auto-merge') + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + timeout-minutes: 5 + steps: + - name: Enable auto-merge + env: + GH_TOKEN: ${{ github.token }} + PR_URL: ${{ github.event.pull_request.html_url }} + run: | + gh pr review --approve "$PR_URL" || echo "PR approval skipped or failed." + gh pr merge --auto --squash "$PR_URL" diff --git a/.github/workflows/update.yml b/.github/workflows/update.yml new file mode 100644 index 0000000..c1f9d18 --- /dev/null +++ b/.github/workflows/update.yml @@ -0,0 +1,37 @@ +name: Update Flake Locks +on: + schedule: + - cron: "0 0 * * 1" + workflow_dispatch: +jobs: + update: + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Generate App token + id: app-token + uses: actions/create-github-app-token@v3 + with: + client-id: ${{ vars.APP_CLIENT_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + - name: Install Nix + uses: DeterminateSystems/nix-installer-action@v22 + - name: Setup Nix Cache + uses: DeterminateSystems/magic-nix-cache-action@v13 + with: + use-flakehub: disabled + use-gha-cache: enabled + - name: Update root flake.lock + uses: DeterminateSystems/update-flake-lock@v28 + with: + token: ${{ steps.app-token.outputs.token }} + pr-title: "chore(flake): update flake.lock" + pr-labels: | + dependencies + auto-merge + commit-msg: "chore(flake): update flake.lock" \ No newline at end of file diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..8563548 --- /dev/null +++ b/.gitignore @@ -0,0 +1,10 @@ +/target +Cargo.lock + +# Nix build outputs and direnv's cached shell. flake.lock IS committed. +result +result-* +.direnv/ + +# macOS +.DS_Store diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 0000000..5680987 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,40 @@ +[package] +name = "sqlx-query" +version = "0.0.0" +edition = "2024" +# Not a choice: sqlx 0.9 declares it, and the tests build against it. The +# library itself would compile on far older, but a crate in this set that +# claimed a lower MSRV than its siblings would be a promise nothing checks. +rust-version = "1.94" +license = "MIT" +description = "Splices SQL fragments into the sentinel comments of a query, for sqlx." +repository = "https://github.com/sqlx-contrib/sqlx-query" +keywords = ["sql", "sqlx", "postgres", "query", "filter"] +categories = ["database"] + +# Denied rather than warned, because several consumers in this ecosystem deny +# pedantic at the workspace level: a lint this crate tolerates is a lint they +# cannot. +[lints.clippy] +all = { level = "deny", priority = -1 } +pedantic = { level = "deny", priority = -1 } + +# No dependencies, deliberately. This crate takes a `&str` and returns a +# `String`; it does not need sqlx to do that, and a caller splicing into a +# hand-written query should not acquire a database driver for the privilege. +# The name says where it belongs, not what it links against. +[dependencies] + +[dev-dependencies] +# For tests/postgres.rs alone -- the round trip is the only thing here that +# needs a database, and it is the only thing text assertions cannot stand in +# for. +sqlx = { version = "0.9", default-features = false, features = [ + "postgres", + "runtime-tokio", +] } +tokio = { version = "1", features = ["macros", "rt-multi-thread"] } + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..c43d20f --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 sqlx-contrib + +Permission is hereby granted, free of charge, to any person obtaining a copy of +this software and associated documentation files (the "Software"), to deal in +the Software without restriction, including without limitation the rights to +use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies +of the Software, and to permit persons to whom the Software is furnished to do +so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 687bb52..7ff479b 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,162 @@ # sqlx-query -Splices SQL fragments into the sentinel comments of a query you already wrote, for sqlx. + +> Splice a SQL fragment into the sentinel comment of a query you already wrote — +> the connective stays with the statement, and unspliced it still runs. + +[![CI](https://github.com/sqlx-contrib/sqlx-query/actions/workflows/ci.yml/badge.svg)](https://github.com/sqlx-contrib/sqlx-query/actions/workflows/ci.yml) +[![Crate](https://img.shields.io/crates/v/sqlx-query)](https://crates.io/crates/sqlx-query) +[![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](LICENSE) + +Substitutes SQL fragments into the comments of a statement, for +[sqlx](https://github.com/launchbadge/sqlx). + +The Rust counterpart of [pgxquery](https://github.com/pgx-contrib/pgxquery), and +the third of the three: [sqlx-cel](https://github.com/sqlx-contrib/sqlx-cel) +makes a fragment out of a CEL expression, +[sqlx-aip](https://github.com/sqlx-contrib/sqlx-aip) makes two out of an AIP +`List` request, and this puts one in a query. It depends on neither — a +`format!` produces a fragment just as well. + +```rust +use sqlx_query::splice; + +const LIST_VOLUMES: &str = "\ +SELECT * FROM volumes +WHERE /* query.where AND */ TRUE +ORDER BY /* query.order_by , */ id +LIMIT $1 OFFSET $2"; + +let sql = splice(LIST_VOLUMES, &[ + ("where", Some(r#""title" = $3"#)), + ("order_by", Some(r#""created_at" DESC"#)), +])?; + +// SELECT * FROM volumes +// WHERE "title" = $3 AND TRUE +// ORDER BY "created_at" DESC , id +// LIMIT $1 OFFSET $2 +``` + +## The connective belongs to the statement + +Whatever else is inside the sentinel is kept, on the side it was written: +`/* query.where AND */` substitutes to ` AND`, and +`/* query.order_by , */` to ` ,`. + +That is the whole trick. The author of the statement decides how a fragment +joins to what surrounds it, so a fragment never has to know — the same one +splices into a `WHERE` that is `AND`ed and one that is `OR`ed, and into an +`ORDER BY` ahead of a primary-key tiebreaker. + +```rust +splice("WHERE archived /* OR query.where */", &[("where", Some("a = $1"))])?; +// WHERE archived OR a = $1 +``` + +## Unspliced, it is a comment + +A sentinel whose fragment is `None` is removed, connective and all, which leaves +`WHERE TRUE` on an unfiltered list. And a statement nobody splices at all runs +exactly as written, sentinels included — they are comments. + +That is what makes the convention safe to put in checked-in or generated SQL. +The file stays valid for `psql`, for `sqlc`, and for whatever else reads it. + +## What is an error, and what is not + +A sentinel this call says nothing about is **left alone**. Substituting what you +were not given would be deciding the statement is wrong. + +A fragment with **no sentinel to go into is an error**. The predicate would +silently not apply, and a dropped predicate widens a result set rather than +emptying it — the kind of bug that returns plausible rows and reaches +production. `Error::MissingSentinel` names the fragment instead. + +## Placeholders + +A spliced fragment lands among the statement's own parameters, and the two have +to agree. Two ways, and the good one costs nothing: + +**Ask the producer to start where the statement stops.** sqlx-cel's `Options` +and sqlx-aip's `rewrite_with` both take a `param_offset`, and +`placeholder_count` is what you pass them: + +```rust +let offset = sqlx_query::placeholder_count(LIST_VOLUMES) + 1; // 3 +``` + +**Or renumber afterwards** with `shift`, for a fragment that arrived numbered +from `$1` and cannot be asked to start elsewhere: + +```rust +assert_eq!(sqlx_query::shift(r#""title" = $1"#, 2), r#""title" = $3"#); +``` + +Both read the SQL properly rather than reaching for a regex. A `$1` inside a +string literal, a quoted identifier, a comment or a dollar-quoted body is text, +not a parameter, and renumbering it produces SQL that still parses and binds the +wrong value: + +```rust +// The literal is left alone; only the parameter moves. +assert_eq!( + sqlx_query::shift("note = 'costs $9' AND id = $1", 4), + "note = 'costs $9' AND id = $5", +); +``` + +## Positional dialects + +All of the above assumes numbered placeholders. With SQLite's or MySQL's `?`, +binds match the *text* rather than a number, so a fragment spliced into the +middle of a statement needs its values bound in the middle of the list too. +`shift` has nothing to do there and `placeholder_count` returns zero. + +Splicing still works; the bookkeeping moves to you. Splice at the end, use a +numbered dialect, or count the placeholders either side of the sentinel +yourself. + +## Is this safe? + +It concatenates strings into SQL, so: exactly as safe as what you hand it. A +fragment from sqlx-cel or sqlx-aip carries literals as placeholders and column +names from a fail-closed allow-list, and is safe to splice. A fragment built by +interpolating a request field is an injection, and nothing here changes that. +sqlx says the same by making you write `AssertSqlSafe` around the result, which +is a sentence you are asserting rather than a cast. + +## Scope + +**In.** Substituting named fragments into sentinel comments. Counting a +statement's placeholders. Renumbering a fragment's. + +**Out.** Building SQL, knowing what a `WHERE` clause is, binding values +(sqlx-cel's `BindAll` does that), talking to a database, and parsing SQL beyond +knowing where text ends. + +The crate has no dependencies — not even sqlx. It takes a `&str` and returns a +`String`. + +## Development + +sqlx 0.9 declares `rust-version = "1.94"`, so this crate does too. +`rust-toolchain.toml` pins the dev toolchain to 1.95.0, so plain `cargo` picks +the right one even when the machine's default stable is older than the MSRV. + +```sh +cargo test +cargo clippy --all-targets +``` + +`tests/postgres.rs` needs a database and skips without `DATABASE_URL`. It is +where the claim that a spliced statement *runs* is checked — text assertions +cannot tell a query bound one slot out from a correct one, because both return +rows. + +There is a Nix flake and a devcontainer for a batteries-included shell — the +pinned toolchain, `psql`, and a Postgres to run against: + +```sh +devcontainer up --workspace-folder . # brings up Postgres +nix develop +``` diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000..c212702 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +# `..` keeps clippy's defaults and adds the product names this crate's docs +# name in prose. Without these, `doc_markdown` wants backticks around every +# mention of a database. +doc-valid-idents = ["SQLite", "PostgreSQL", "MySQL", "MariaDB", ".."] diff --git a/flake.lock b/flake.lock new file mode 100644 index 0000000..a27e98b --- /dev/null +++ b/flake.lock @@ -0,0 +1,173 @@ +{ + "nodes": { + "devcontainer-env": { + "inputs": { + "flake-utils": "flake-utils", + "nixpkgs": "nixpkgs", + "rust-overlay": "rust-overlay" + }, + "locked": { + "lastModified": 1788752194, + "narHash": "sha256-2374dcsCZ/ghyZvVGDFefDh57RfTPB5k3l9v0lwWHok=", + "owner": "devcontainer-env", + "repo": "devcontainer-env", + "rev": "14f4ddc6df1b3b5ad690f10d0082fe574a53fa78", + "type": "github" + }, + "original": { + "owner": "devcontainer-env", + "repo": "devcontainer-env", + "type": "github" + } + }, + "flake-utils": { + "inputs": { + "systems": "systems" + }, + "locked": { + "lastModified": 1731533236, + "narHash": "sha256-l0KFg5HjrsfsO/JpG+r7fRrqm12kzFHyUHqHCVpMMbI=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "11707dc2f618dd54ca8739b309ec4fc024de578b", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "flake-utils_2": { + "inputs": { + "systems": "systems_2" + }, + "locked": { + "lastModified": 1731533236, + "narHash": "sha256-l0KFg5HjrsfsO/JpG+r7fRrqm12kzFHyUHqHCVpMMbI=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "11707dc2f618dd54ca8739b309ec4fc024de578b", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1788549839, + "narHash": "sha256-kOrCcSIA6w9J1hX5DqHy2k9pDTJymExTsbV74U9UtCA=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "17de0b976395537756f30a3e78f2f06e5cec89ed", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixpkgs-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "nixpkgs_2": { + "locked": { + "lastModified": 1788549839, + "narHash": "sha256-kOrCcSIA6w9J1hX5DqHy2k9pDTJymExTsbV74U9UtCA=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "17de0b976395537756f30a3e78f2f06e5cec89ed", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixpkgs-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "root": { + "inputs": { + "devcontainer-env": "devcontainer-env", + "flake-utils": "flake-utils_2", + "nixpkgs": "nixpkgs_2", + "rust-overlay": "rust-overlay_2" + } + }, + "rust-overlay": { + "inputs": { + "nixpkgs": [ + "devcontainer-env", + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1788678114, + "narHash": "sha256-pcqbpV4ZI79al6KAlr+WJjaEo/PYfgctRlG43D5BSJM=", + "owner": "oxalica", + "repo": "rust-overlay", + "rev": "4748ec2f5ed4a881474ed4c98aa71a5308cdac8d", + "type": "github" + }, + "original": { + "owner": "oxalica", + "repo": "rust-overlay", + "type": "github" + } + }, + "rust-overlay_2": { + "inputs": { + "nixpkgs": [ + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1788678114, + "narHash": "sha256-pcqbpV4ZI79al6KAlr+WJjaEo/PYfgctRlG43D5BSJM=", + "owner": "oxalica", + "repo": "rust-overlay", + "rev": "4748ec2f5ed4a881474ed4c98aa71a5308cdac8d", + "type": "github" + }, + "original": { + "owner": "oxalica", + "repo": "rust-overlay", + "type": "github" + } + }, + "systems": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + }, + "systems_2": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 0000000..8a47c13 --- /dev/null +++ b/flake.nix @@ -0,0 +1,77 @@ +{ + description = "sqlx-query — splices SQL fragments into the sentinel comments of a query, for sqlx"; + + inputs = { + nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable"; + flake-utils.url = "github:numtide/flake-utils"; + rust-overlay = { + url = "github:oxalica/rust-overlay"; + inputs.nixpkgs.follows = "nixpkgs"; + }; + devcontainer-env.url = "github:devcontainer-env/devcontainer-env"; + }; + + outputs = + { + nixpkgs, + flake-utils, + rust-overlay, + devcontainer-env, + ... + }: + flake-utils.lib.eachDefaultSystem ( + system: + let + pkgs = import nixpkgs { + inherit system; + overlays = [ (import rust-overlay) ]; + }; + manifest = (pkgs.lib.importTOML ./Cargo.toml).package; + rust-toolchain = pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml; + in + { + # No `packages.default`. This is a library crate with no binary, and + # `buildRustPackage` would want a committed Cargo.lock -- which a library + # deliberately does not have. The dev shell is the whole point here. + devShells.default = pkgs.mkShell { + inherit (manifest) name; + + packages = with pkgs; [ + rust-toolchain + pkg-config + # A CLI only, for poking at what the tests leave behind. The + # Postgres tests/postgres.rs runs against is the compose service in + # .devcontainer, not a server started here. + postgresql + devcontainer-env.packages.${system}.default + ]; + + # DATABASE_URL is defined once, in .devcontainer/devcontainer.json. + # `export` reads it from `containerEnv` and rewrites the compose + # hostname to the port Docker assigned, so the same definition is + # correct inside the container, on the host, and on a CI runner. + # + # Tolerating failure is deliberate: with no devcontainer running -- + # a contributor without Docker, or someone only touching the unit + # tests -- DATABASE_URL stays unset and tests/postgres.rs skips, + # which is its documented behaviour. + shellHook = '' + eval "$(devcontainer-env export 2>/dev/null)" 2>/dev/null || true + + # Only greet a human. CI drives this shell with + # `nix develop --command ...`, where a banner is just noise in front + # of the output someone is actually reading. + if [[ $- == *i* ]]; then + echo "${manifest.name} ${manifest.version} — $(cargo --version)" + if [[ -n "''${DATABASE_URL:-}" ]]; then + echo " cargo test # incl. the end-to-end Postgres round trip" + else + echo " cargo test # Postgres tests skip; start the devcontainer for them" + fi + echo " cargo clippy --all-targets" + fi + ''; + }; + } + ); +} diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000..8138e8f --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,8 @@ +[toolchain] +# The MSRV in Cargo.toml is 1.94, which sqlx 0.9 forces. This is the *dev* +# toolchain and is deliberately newer: pinning it means `cargo` picks the right +# one here even when the machine's default stable is older than the MSRV, which +# otherwise fails with a resolver error naming five packages at once. +channel = "1.95.0" +components = ["rustfmt", "clippy", "rust-src", "rust-analyzer"] +profile = "minimal" diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 0000000..2609338 --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,421 @@ +//! Splices SQL fragments into the sentinel comments of a query you already +//! wrote, for [sqlx](https://github.com/launchbadge/sqlx). +//! +//! ``` +//! use sqlx_query::splice; +//! +//! const LIST_VOLUMES: &str = "\ +//! SELECT * FROM volumes +//! WHERE /* query.where AND */ TRUE +//! ORDER BY /* query.order_by , */ id +//! LIMIT $1 OFFSET $2"; +//! +//! let sql = splice(LIST_VOLUMES, &[ +//! ("where", Some(r#""title" = $3"#)), +//! ("order_by", Some(r#""created_at" DESC"#)), +//! ])?; +//! +//! assert!(sql.contains(r#"WHERE "title" = $3 AND TRUE"#)); +//! assert!(sql.contains(r#"ORDER BY "created_at" DESC , id"#)); +//! # Ok::<_, sqlx_query::Error>(()) +//! ``` +//! +//! # What this crate is +//! +//! Text substitution, and a convention. A statement carries a comment where a +//! fragment may go; this puts one there. It does not build SQL, does not know +//! what a `WHERE` clause is, does not talk to a database, and has no +//! dependencies — not even sqlx. +//! +//! It is the Rust counterpart of +//! [pgxquery](https://github.com/pgx-contrib/pgxquery), which does the same job +//! at a different moment: pgx exposes a `QueryRewriter` hook, so there the +//! substitution happens as the query is sent and the caller never sees it. sqlx +//! has no such hook — `query_as` takes a string and binds positionally — so the +//! substitution has to happen where the string is built, and it is a function +//! rather than an interface. +//! +//! Where the fragments come from is not this crate's business. +//! [sqlx-cel](https://github.com/sqlx-contrib/sqlx-cel) transpiles a CEL filter +//! into one, [sqlx-aip](https://github.com/sqlx-contrib/sqlx-aip) turns a whole +//! AIP `List` request into two, and a `format!` will do. All this needs is a +//! string. +//! +//! # The sentinel +//! +//! A sentinel is a block comment naming `query.`: +//! +//! ```sql +//! WHERE +//! /* query.where AND */ TRUE +//! ORDER BY +//! /* query.order_by , */ id -- primary key, so the order is total +//! ``` +//! +//! Whatever else is inside the comment is kept, on the side it was written: +//! `/* query.where AND */` substitutes to ` AND`, and +//! `/* query.order_by , */` to ` ,`. **The connective belongs to the +//! SQL, not to the fragment.** That is the whole trick — the author of the +//! statement decides how a fragment joins to what surrounds it, so a fragment +//! never has to know, and the same fragment can be spliced into a `WHERE` that +//! is `AND`ed and one that is `OR`ed. +//! +//! A sentinel whose fragment is [`None`] is removed, comment and connective +//! together, which is what leaves `WHERE TRUE` on an unfiltered list. A +//! statement with no fragments at all therefore runs exactly as written, which +//! is what makes the convention safe to put in generated SQL: unspliced, it is +//! a comment. +//! +//! A comment naming something not in the list — `/* query.limit */` when only +//! `where` was supplied — is left alone. This crate substitutes what it was +//! given and does not decide that a sentinel is stale. +//! +//! The reverse is an error. A fragment supplied for a sentinel the statement +//! does not carry means the predicate silently would not apply, and a dropped +//! predicate widens a result set rather than emptying it, so it fails loudly as +//! [`Error::MissingSentinel`] instead. +//! +//! # Placeholders +//! +//! A spliced fragment lands in a statement that usually binds parameters of its +//! own, and the two sets have to agree. There are two ways to arrange that, and +//! the good one costs nothing: +//! +//! **Ask the producer to start where the statement stops.** Both sqlx-cel and +//! sqlx-aip take a `param_offset`, and [`placeholder_count`] is how you know +//! what to pass: +//! +//! ``` +//! # use sqlx_query::placeholder_count; +//! # const LIST_VOLUMES: &str = "SELECT * FROM volumes LIMIT $1 OFFSET $2"; +//! let offset = placeholder_count(LIST_VOLUMES) + 1; // 3 +//! # assert_eq!(offset, 3); +//! ``` +//! +//! **Or renumber the fragment afterwards**, with [`shift`], for a fragment that +//! arrived numbered from `$1` and cannot be asked to start elsewhere. It is +//! correct, and it re-reads the SQL to do a job that need not have existed. +//! +//! Either way, the values are bound in the order the numbers say: the +//! statement's own first, the fragment's after them. +//! +//! # Positional dialects +//! +//! Everything above assumes numbered placeholders. With SQLite's or MySQL's +//! `?`, binds are matched to the *text* rather than to a number, so a fragment +//! spliced into the middle of a statement must have its values bound in the +//! middle of the list too — after the values of the placeholders before it, and +//! before those after it. [`shift`] has nothing to do there and +//! [`placeholder_count`] returns zero. +//! +//! Splicing still works; the bookkeeping moves to the caller. Splice at the end +//! of a statement, or use a numbered dialect, or count the placeholders on each +//! side of the sentinel yourself. +//! +//! # Is this safe? +//! +//! It concatenates strings into SQL, so the honest answer is: exactly as safe as +//! what you hand it. A fragment from sqlx-cel or sqlx-aip contains literals as +//! placeholders and column names from a fail-closed allow-list, and is safe to +//! splice. A fragment built by interpolating a request field is an injection, +//! and no amount of care here changes that. +//! +//! sqlx says the same thing by making you write `AssertSqlSafe` around the +//! result, which is a sentence you are asserting rather than a cast. + +#![deny(missing_docs)] +#![cfg_attr(docsrs, feature(doc_cfg))] + +mod scan; + +pub use scan::{placeholder_count, shift}; + +use core::fmt; + +/// The prefix that marks a comment as a sentinel. +/// +/// Deliberately not configurable. The point of a convention is that a statement +/// written for one project splices in another, and pgxquery has already spelled +/// it this way. +const PREFIX: &str = "query."; + +/// Substitutes `fragments` into the sentinel comments of `sql`. +/// +/// Each entry is a sentinel name — the part after `query.` — and the text to +/// put there, or [`None`] to remove the sentinel. See the crate docs for the +/// convention. +/// +/// ``` +/// # use sqlx_query::splice; +/// let sql = splice( +/// "SELECT * FROM t WHERE /* query.where AND */ TRUE", +/// &[("where", Some("a = $1"))], +/// )?; +/// +/// assert_eq!(sql, "SELECT * FROM t WHERE a = $1 AND TRUE"); +/// # Ok::<_, sqlx_query::Error>(()) +/// ``` +/// +/// # Errors +/// +/// [`Error::MissingSentinel`] when a fragment is [`Some`] and `sql` has no +/// sentinel to put it in — which would otherwise drop a predicate and widen the +/// result set, silently. +pub fn splice(sql: &str, fragments: &[(&str, Option<&str>)]) -> Result { + let mut spliced = String::with_capacity(sql.len()); + let mut rest = sql; + // Which sentinels were actually found, so the check below can tell a + // fragment that was used from one that had nowhere to go. + let mut substituted = vec![false; fragments.len()]; + + while let Some(open) = rest.find("/*") { + let Some(length) = rest[open..].find("*/") else { + // Unterminated, so there is no comment here to substitute into and + // nothing further to find. The database can have its opinion. + break; + }; + let close = open + length + "*/".len(); + + spliced.push_str(&rest[..open]); + + match sentinel(&rest[open + "/*".len()..close - "*/".len()]) { + Some((prefix, name, suffix)) => match position(fragments, name) { + Some(index) => { + substituted[index] = true; + if let Some(fragment) = fragments[index].1 { + spliced.push_str(prefix); + spliced.push_str(fragment); + spliced.push_str(suffix); + } + } + // A sentinel this call says nothing about. Left as it was: + // it is a comment, and the statement runs with it. + None => spliced.push_str(&rest[open..close]), + }, + None => spliced.push_str(&rest[open..close]), + } + + rest = &rest[close..]; + } + + spliced.push_str(rest); + + for (index, (name, fragment)) in fragments.iter().enumerate() { + if fragment.is_some() && !substituted[index] { + return Err(Error::MissingSentinel { + name: (*name).to_owned(), + }); + } + } + + Ok(spliced) +} + +/// Splits a comment body around the `query.` it holds. +/// +/// Returns the text either side, with the whitespace that abutted the comment +/// markers trimmed off, so `" query.where AND "` yields `("", " AND")` and the +/// substitution reads ` AND`. +/// +/// The name runs to the first character that cannot be part of one, so +/// `query.where` and `query.order_by` are both found without the caller +/// declaring which names exist. +fn sentinel(body: &str) -> Option<(&str, &str, &str)> { + let at = body.find(PREFIX)?; + let after = at + PREFIX.len(); + + let length = body[after..] + .find(|character: char| !character.is_ascii_alphanumeric() && character != '_') + .unwrap_or(body.len() - after); + if length == 0 { + return None; + } + + Some(( + body[..at].trim_start(), + &body[after..after + length], + body[after + length..].trim_end(), + )) +} + +/// Finds `name` among the supplied fragments. +/// +/// A linear scan: the list is two entries long in every real call, and this +/// keeps the parameter an ordinary slice rather than something the caller has +/// to build. +fn position(fragments: &[(&str, Option<&str>)], name: &str) -> Option { + fragments + .iter() + .position(|(candidate, _)| *candidate == name) +} + +/// Why a [`splice`] failed. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum Error { + /// A fragment was supplied for a sentinel the statement does not carry. + /// + /// Almost always a typo in one of the two — `/* query.wehre AND */`, or a + /// name that was renamed on one side only. It is an error rather than a + /// no-op because the alternative is a filter that quietly does not apply, + /// and a query that returns *more* rows than it should is the kind of bug + /// that reaches production. + MissingSentinel { + /// The name that had nowhere to go. + name: String, + }, +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::MissingSentinel { name } => write!( + f, + "no /* query.{name} … */ sentinel in the statement to splice the {name} fragment into", + ), + } + } +} + +impl core::error::Error for Error {} + +#[cfg(test)] +mod tests { + use super::{Error, splice}; + + /// The shape sqlc emits, sentinels and all. + const LIST: &str = "SELECT *\nFROM volumes\nWHERE\n /* query.where AND */ TRUE\nORDER BY\n /* query.order_by , */ id\nLIMIT $1 OFFSET $2"; + + fn list(where_sql: Option<&str>, order_sql: Option<&str>) -> String { + splice(LIST, &[("where", where_sql), ("order_by", order_sql)]).unwrap() + } + + #[test] + fn substitutes_both_and_keeps_the_connectives_where_they_were_written() { + let sql = list(Some(r#""title" = $3"#), Some(r#""created_at" DESC"#)); + + assert!(sql.contains("WHERE\n \"title\" = $3 AND TRUE"), "{sql}"); + assert!( + sql.contains("ORDER BY\n \"created_at\" DESC , id"), + "{sql}" + ); + } + + /// The property that makes the convention safe to generate: unspliced, the + /// statement is the statement. + #[test] + fn no_fragments_leaves_the_statement_running_as_written() { + let sql = list(None, None); + + assert!(!sql.contains("query."), "{sql}"); + assert!(sql.contains("WHERE\n TRUE"), "{sql}"); + assert!(sql.contains("ORDER BY\n id"), "{sql}"); + } + + #[test] + fn one_fragment_does_not_disturb_the_others_sentinel() { + let sql = list(Some("a = $3"), None); + + assert!(sql.contains("WHERE\n a = $3 AND TRUE"), "{sql}"); + assert!(sql.contains("ORDER BY\n id"), "{sql}"); + } + + /// The connective is the statement's, so the same fragment reads correctly + /// in a query that joins it differently. + #[test] + fn the_statement_decides_how_a_fragment_joins() { + let sql = splice( + "WHERE archived /* OR query.where */", + &[("where", Some("a = $1"))], + ) + .unwrap(); + + assert_eq!(sql, "WHERE archived OR a = $1"); + } + + #[test] + fn a_comment_that_is_not_a_sentinel_is_left_alone() { + let sql = splice( + "SELECT 1 /* an ordinary comment */ WHERE /* query.where AND */ TRUE", + &[("where", Some("a = $1"))], + ) + .unwrap(); + + assert_eq!( + sql, + "SELECT 1 /* an ordinary comment */ WHERE a = $1 AND TRUE" + ); + } + + /// A sentinel the call says nothing about survives: substituting what you + /// were not given would be deciding the statement is wrong. + #[test] + fn an_unmentioned_sentinel_survives() { + let sql = splice( + "WHERE /* query.where AND */ TRUE LIMIT /* query.limit */ 10", + &[("where", Some("a = $1"))], + ) + .unwrap(); + + assert_eq!(sql, "WHERE a = $1 AND TRUE LIMIT /* query.limit */ 10"); + } + + /// The reverse, which is not survivable: the filter would silently not + /// apply and the page would be wider than the caller asked for. + #[test] + fn a_fragment_with_nowhere_to_go_is_an_error() { + let error = splice("SELECT * FROM t", &[("where", Some("a = $1"))]).unwrap_err(); + + assert_eq!( + error, + Error::MissingSentinel { + name: "where".to_owned() + }, + ); + assert!(error.to_string().contains("query.where"), "{error}"); + } + + /// Nothing to splice, nothing to warn about. + #[test] + fn an_absent_fragment_with_nowhere_to_go_is_fine() { + let sql = splice("SELECT * FROM t", &[("where", None)]).unwrap(); + + assert_eq!(sql, "SELECT * FROM t"); + } + + #[test] + fn a_sentinel_may_appear_more_than_once() { + let sql = splice( + "WHERE /* query.where AND */ TRUE UNION SELECT * FROM u WHERE /* query.where AND */ TRUE", + &[("where", Some("a = $1"))], + ) + .unwrap(); + + assert_eq!(sql.matches("a = $1").count(), 2, "{sql}"); + } + + #[test] + fn an_unterminated_comment_is_left_where_it_is() { + let sql = splice( + "WHERE /* query.where AND */ TRUE /* unterminated", + &[("where", Some("a = $1"))], + ) + .unwrap(); + + assert_eq!(sql, "WHERE a = $1 AND TRUE /* unterminated"); + } + + #[test] + fn a_bare_prefix_is_not_a_sentinel() { + let sql = splice("SELECT 1 /* query. */", &[("where", None)]).unwrap(); + + assert_eq!(sql, "SELECT 1 /* query. */"); + } + + #[test] + fn a_statement_with_no_comments_at_all_is_returned_whole() { + let sql = splice("SELECT 1", &[]).unwrap(); + + assert_eq!(sql, "SELECT 1"); + } +} diff --git a/src/scan.rs b/src/scan.rs new file mode 100644 index 0000000..f080ee7 --- /dev/null +++ b/src/scan.rs @@ -0,0 +1,375 @@ +//! A scanner over SQL text, and the two things this crate reads off it. +//! +//! Both [`placeholder_count`] and [`shift`] have to answer the same question — +//! *is this `$1` a placeholder, or is it text?* — so both walk the statement the +//! same way, stepping over the five constructs where a `$` means nothing: +//! +//! | | | +//! | --- | --- | +//! | `'…'` | a string literal, `''` escaping a quote | +//! | `"…"` | a quoted identifier, `""` escaping a quote | +//! | `-- …` | a line comment | +//! | `/* … */` | a block comment, which PostgreSQL allows to nest | +//! | `$tag$…$tag$` | a dollar-quoted string | +//! +//! Every one of those can contain a `$1`, and none of them binds anything. A +//! naive scan over `WHERE note = 'costs $1' AND id = $1` sees two placeholders +//! and renumbers the wrong one, which is a bug that survives review because the +//! SQL still parses. +//! +//! This is not a SQL parser and does not try to be. It knows where text ends, +//! which is the whole of what these two functions need. + +/// Returns the highest `$N` in `sql`, or `0` when it binds nothing. +/// +/// This is how many parameters a statement already has, which is what a caller +/// needs to know before splicing a fragment into it: the fragment's first +/// placeholder is this plus one. +/// +/// The *highest*, not the count of occurrences. `$1` may be referenced from +/// several places and bound once, so counting occurrences would over-report; +/// and a statement is free to skip a number, in which case the driver still +/// expects that many values. The highest is the only answer that is right for +/// both. +/// +/// ``` +/// assert_eq!(sqlx_query::placeholder_count("SELECT * FROM t WHERE a = $1 AND b = $2"), 2); +/// assert_eq!(sqlx_query::placeholder_count("SELECT * FROM t"), 0); +/// // Text is not a placeholder, however much it looks like one. +/// assert_eq!(sqlx_query::placeholder_count("SELECT '$9' FROM t WHERE a = $1"), 1); +/// ``` +#[must_use] +pub fn placeholder_count(sql: &str) -> usize { + let mut highest = 0; + + scan(sql, |token| { + if let Token::Placeholder(number) = token { + highest = highest.max(number); + } + }); + + highest +} + +/// Renumbers every placeholder in `sql` by `offset`, so `$1` becomes +/// `$(1 + offset)`. +/// +/// For a fragment that arrived numbered from `$1` and has to be spliced into a +/// statement that already binds parameters. **Prefer not needing it**: a +/// producer that accepts a starting offset — [`sqlx-cel`]'s `Options` and +/// [`sqlx-aip`]'s `rewrite_with`, both of which take one — emits the right +/// numbers to begin with, and then nothing has to re-read the SQL at all. Use +/// this for a fragment you were handed and cannot ask to renumber. +/// +/// ``` +/// // The statement binds $1 and $2 already, so the fragment starts at $3. +/// assert_eq!(sqlx_query::shift(r#""title" = $1"#, 2), r#""title" = $3"#); +/// ``` +/// +/// Only numbered placeholders move. A positional `?` has no number and is +/// returned untouched — see the crate docs on what that costs. +/// +/// [`sqlx-cel`]: https://github.com/sqlx-contrib/sqlx-cel +/// [`sqlx-aip`]: https://github.com/sqlx-contrib/sqlx-aip +#[must_use] +pub fn shift(sql: &str, offset: usize) -> String { + if offset == 0 { + return sql.to_owned(); + } + + let mut shifted = String::with_capacity(sql.len()); + + scan(sql, |token| match token { + Token::Placeholder(number) => { + shifted.push('$'); + shifted.push_str(&(number + offset).to_string()); + } + Token::Text(text) => shifted.push_str(text), + }); + + shifted +} + +/// What [`scan`] hands its visitor. +enum Token<'a> { + /// A `$N`, already parsed. The text it came from is not passed, because + /// every caller that wants it wants it renumbered. + Placeholder(usize), + /// Everything else, in runs as long as the scanner can make them. + Text(&'a str), +} + +/// Walks `sql`, calling `visit` for each placeholder and each run of text +/// between them. +/// +/// The concatenation of every `Token::Text` and the source of every +/// `Token::Placeholder` is exactly `sql`, which is what lets [`shift`] rebuild +/// the statement by appending as it goes. +fn scan<'a, F>(sql: &'a str, mut visit: F) +where + F: FnMut(Token<'a>), +{ + let bytes = sql.as_bytes(); + // The start of the current run of ordinary text, flushed whenever the + // scanner reaches something it has to treat specially. + let mut text = 0; + let mut at = 0; + + while at < bytes.len() { + // Each arm returns where the construct ends. `None` means "ordinary + // text", and the byte is simply consumed. + let skipped = match bytes[at] { + b'\'' => Some(quoted(bytes, at, b'\'')), + b'"' => Some(quoted(bytes, at, b'"')), + b'-' if bytes.get(at + 1) == Some(&b'-') => Some(line_comment(bytes, at)), + b'/' if bytes.get(at + 1) == Some(&b'*') => Some(block_comment(bytes, at)), + b'$' => { + if let Some((number, end)) = placeholder(bytes, at) { + if text < at { + visit(Token::Text(&sql[text..at])); + } + visit(Token::Placeholder(number)); + text = end; + at = end; + continue; + } + // Not `$N`, so either dollar-quoting or a lone `$`. + dollar_quoted(bytes, at) + } + _ => None, + }; + + at = match skipped { + Some(end) => end, + None => at + 1, + }; + } + + if text < bytes.len() { + visit(Token::Text(&sql[text..])); + } +} + +/// The end of a `'…'` or `"…"` beginning at `at`, doubled quotes included. +/// +/// An unterminated literal ends at the end of the input rather than being an +/// error: this scanner reports what it can see, and a statement that does not +/// parse is the database's to complain about. +fn quoted(bytes: &[u8], at: usize, quote: u8) -> usize { + let mut index = at + 1; + + while index < bytes.len() { + if bytes[index] == quote { + // A doubled quote is an escaped one, and the literal continues. + if bytes.get(index + 1) == Some("e) { + index += 2; + continue; + } + return index + 1; + } + index += 1; + } + + bytes.len() +} + +/// The end of a `-- …` comment, including its newline. +fn line_comment(bytes: &[u8], at: usize) -> usize { + let mut index = at + 2; + + while index < bytes.len() && bytes[index] != b'\n' { + index += 1; + } + + (index + 1).min(bytes.len()) +} + +/// The end of a `/* … */` comment, honouring PostgreSQL's nesting. +/// +/// Nesting is why this cannot be a search for the next `*/`: in +/// `/* a /* b */ c */` that would stop in the middle and leave `c */` to be +/// scanned as SQL. +fn block_comment(bytes: &[u8], at: usize) -> usize { + let mut index = at + 2; + let mut depth = 1usize; + + while index < bytes.len() && depth > 0 { + if bytes[index] == b'/' && bytes.get(index + 1) == Some(&b'*') { + depth += 1; + index += 2; + } else if bytes[index] == b'*' && bytes.get(index + 1) == Some(&b'/') { + depth -= 1; + index += 2; + } else { + index += 1; + } + } + + index +} + +/// The number and end of a `$N` beginning at `at`, if that is what it is. +fn placeholder(bytes: &[u8], at: usize) -> Option<(usize, usize)> { + let mut index = at + 1; + + while index < bytes.len() && bytes[index].is_ascii_digit() { + index += 1; + } + + if index == at + 1 { + return None; + } + + // Digits only, and bounded by the length of the statement, so the parse + // cannot fail for any reason but overflow -- at which point the statement + // has bigger problems than this crate. + let number = core::str::from_utf8(&bytes[at + 1..index]) + .ok()? + .parse() + .ok()?; + + Some((number, index)) +} + +/// The end of a `$tag$…$tag$` string beginning at `at`, if that is what it is. +/// +/// The tag is empty (`$$…$$`) or an identifier, and the closing tag must match +/// it exactly. An unclosed one runs to the end of the input, on the same +/// reasoning as [`quoted`]. +fn dollar_quoted(bytes: &[u8], at: usize) -> Option { + let mut index = at + 1; + + while index < bytes.len() && bytes[index] != b'$' { + let character = bytes[index]; + let valid = character.is_ascii_alphabetic() + || character == b'_' + // A digit is allowed inside a tag but not as its first character, + // which is also what stops `$1` reaching here as a tag. + || (index > at + 1 && character.is_ascii_digit()); + if !valid { + return None; + } + index += 1; + } + + if bytes.get(index) != Some(&b'$') { + return None; + } + + let tag = &bytes[at..=index]; + let body = index + 1; + + let close = bytes[body..] + .windows(tag.len()) + .position(|window| window == tag); + + Some(match close { + Some(offset) => body + offset + tag.len(), + None => bytes.len(), + }) +} + +#[cfg(test)] +mod tests { + use super::{placeholder_count, shift}; + + #[test] + fn counts_the_highest_placeholder_rather_than_the_occurrences() { + // $1 twice and $3 once: three values are expected, not three + // occurrences and not two distinct numbers. + assert_eq!(placeholder_count("a = $1 OR (b = $1 AND c = $3)"), 3); + } + + #[test] + fn a_statement_with_no_placeholders_binds_nothing() { + assert_eq!(placeholder_count("SELECT 1"), 0); + assert_eq!(placeholder_count(""), 0); + } + + #[test] + fn shifts_every_placeholder_and_leaves_the_rest_alone() { + assert_eq!( + shift(r#"("a" = $1 OR "a" = $2) AND "b" > $10"#, 2), + r#"("a" = $3 OR "a" = $4) AND "b" > $12"#, + ); + } + + #[test] + fn a_zero_shift_is_the_statement_unchanged() { + assert_eq!(shift("a = $1", 0), "a = $1"); + } + + /// The reason this is a scanner and not a regex. A `LIKE` fragment carries + /// `'%'` literals, and a literal can carry anything at all. + #[test] + fn text_inside_a_string_literal_is_not_a_placeholder() { + assert_eq!(placeholder_count("note = 'costs $9' AND id = $1"), 1); + assert_eq!( + shift("note = 'costs $9' AND id = $1", 4), + "note = 'costs $9' AND id = $5", + ); + assert_eq!( + shift(r#""a" LIKE '%' || $1 || '%'"#, 1), + r#""a" LIKE '%' || $2 || '%'"#, + ); + } + + #[test] + fn a_doubled_quote_does_not_end_the_literal() { + // The literal is `it's $9`, so the $9 is still text. + assert_eq!(placeholder_count("a = 'it''s $9' AND b = $1"), 1); + assert_eq!(placeholder_count(r#""odd""name $9" = $1"#), 1); + } + + #[test] + fn text_inside_a_comment_is_not_a_placeholder() { + assert_eq!(placeholder_count("a = $1 -- was $9\nAND b = $2"), 2); + assert_eq!(placeholder_count("a = $1 /* was $9 */"), 1); + // Nested, which is where a search for the next `*/` would stop early + // and then scan `$9 */` as SQL. + assert_eq!(placeholder_count("a = $1 /* /* $9 */ $9 */"), 1); + } + + #[test] + fn text_inside_a_dollar_quoted_string_is_not_a_placeholder() { + assert_eq!(placeholder_count("a = $body$ $9 $body$ AND b = $1"), 1); + assert_eq!(placeholder_count("a = $$ $9 $$ AND b = $1"), 1); + assert_eq!( + shift("a = $body$ $9 $body$ AND b = $1", 3), + "a = $body$ $9 $body$ AND b = $4", + ); + } + + /// A tag cannot start with a digit, which is what keeps `$1` a placeholder + /// rather than the opening of a dollar-quoted string. + #[test] + fn a_placeholder_is_not_read_as_a_dollar_quote_tag() { + assert_eq!(placeholder_count("a = $1$ AND b = $2"), 2); + } + + /// Unterminated text runs to the end rather than raising: this is a + /// scanner, and the statement is the database's to reject. + #[test] + fn unterminated_text_swallows_the_rest() { + assert_eq!(placeholder_count("a = 'open $9"), 0); + assert_eq!(placeholder_count("a = $tag$ open $9"), 0); + assert_eq!(placeholder_count("a = $1 /* open $9"), 1); + } + + #[test] + fn a_lone_dollar_is_left_where_it_is() { + assert_eq!(shift("cost = '$' || $1", 1), "cost = '$' || $2"); + assert_eq!(placeholder_count("a = $ AND b = $1"), 1); + } + + /// Multi-byte text is copied whole rather than byte by byte, so an index + /// landing inside a character would panic on the slice. It must not. + #[test] + fn text_outside_ascii_survives() { + assert_eq!( + shift("titel = 'Grüße $9' AND id = $1", 1), + "titel = 'Grüße $9' AND id = $2" + ); + assert_eq!(placeholder_count("titel = '日本語 $9' AND id = $2"), 2); + } +} diff --git a/tests/postgres.rs b/tests/postgres.rs new file mode 100644 index 0000000..5f6144f --- /dev/null +++ b/tests/postgres.rs @@ -0,0 +1,193 @@ +//! End-to-end tests against a real Postgres. +//! +//! The unit tests assert what the spliced statement *says*. These assert the +//! part text cannot: that it parses, and that the placeholders line up with the +//! values across the boundary between the statement's own parameters and the +//! fragment's. Both failures produce SQL that reads correctly — the first is +//! caught by the server, the second by nothing at all, since a query bound one +//! slot out still runs and still returns rows. +//! +//! Set `DATABASE_URL` to run them. Without it each test skips, because a +//! missing database is a missing environment rather than a failure: +//! +//! ```sh +//! DATABASE_URL=postgres://localhost/sqlx_query_test cargo test --test postgres +//! ``` + +use std::str::FromStr as _; + +use sqlx::postgres::PgConnectOptions; +use sqlx::{AssertSqlSafe, PgPool, Row}; +use sqlx_query::{placeholder_count, shift, splice}; + +/// The shape sqlc generates: two parameters of its own, and a sentinel before +/// either of them. +const LIST_VOLUMES: &str = "\ +SELECT title +FROM volumes +WHERE + /* query.where AND */ TRUE +ORDER BY + /* query.order_by , */ id +LIMIT $1 OFFSET $2"; + +/// Creates `schema`, seeds `volumes` inside it, and returns a pool whose +/// `search_path` points there. Returns `None` when `DATABASE_URL` is unset. +/// +/// A schema per test, because `cargo test` runs them concurrently against one +/// database and they would otherwise be seeding the same table. +async fn pool(schema: &str) -> Option { + let Ok(url) = std::env::var("DATABASE_URL") else { + // Skipping is right on a machine with no Docker, but in CI it would + // mean the round trip quietly stopped being tested -- and a skipped + // test looks exactly like a passing one. The devcontainer is there + // precisely so this cannot happen, so assert it rather than trust it. + assert!( + std::env::var_os("CI").is_none(), + "DATABASE_URL is unset in CI: the devcontainer's Postgres never \ + reached the shell, so these tests would have silently skipped", + ); + eprintln!("skipped: DATABASE_URL is unset"); + return None; + }; + + let admin = PgPool::connect(&url) + .await + .expect("DATABASE_URL must connect"); + + // The schema name is this file's, never a caller's, so the format! is not + // an injection -- and an identifier cannot be a bind parameter anyway. + for statement in [ + format!("DROP SCHEMA IF EXISTS {schema} CASCADE"), + format!("CREATE SCHEMA {schema}"), + ] { + sqlx::query(AssertSqlSafe(statement)) + .execute(&admin) + .await + .expect("create the schema"); + } + admin.close().await; + + // `search_path` goes on the *connection options*, so every connection the + // pool opens has it. A `SET search_path` statement would apply to whichever + // pooled connection happened to run it, and the next statement -- taken + // from a different connection -- would create its table in `public`, where + // it collides with every other test doing the same. + let options = PgConnectOptions::from_str(&url) + .expect("DATABASE_URL must parse") + .options([("search_path", schema)]); + let pool = PgPool::connect_with(options) + .await + .expect("connect to the schema"); + + for statement in [ + "CREATE TABLE volumes (id BIGSERIAL PRIMARY KEY, title TEXT NOT NULL, read_count INT NOT NULL)", + "INSERT INTO volumes (title, read_count) VALUES ('Dune', 9), ('Emma', 3), ('Ulysses', 12)", + ] { + sqlx::query(statement) + .execute(&pool) + .await + .expect("seed the schema"); + } + + Some(pool) +} + +#[tokio::test] +async fn a_spliced_filter_runs_and_selects_what_it_says() { + let Some(pool) = pool("splice_filter").await else { + return; + }; + + // The statement binds $1 and $2, so the fragment starts at $3 -- which is + // the number `placeholder_count` exists to produce. + let offset = placeholder_count(LIST_VOLUMES) + 1; + assert_eq!(offset, 3); + + let sql = splice( + LIST_VOLUMES, + &[ + ("where", Some(&format!("read_count > ${offset}"))), + ("order_by", Some("title DESC")), + ], + ) + .expect("splice"); + + let rows = sqlx::query(AssertSqlSafe(sql)) + .bind(10_i64) // $1, LIMIT + .bind(0_i64) // $2, OFFSET + .bind(5_i32) // $3, the fragment's + .fetch_all(&pool) + .await + .expect("the spliced statement must run"); + + let titles: Vec = rows.iter().map(|row| row.get("title")).collect(); + assert_eq!(titles, ["Ulysses", "Dune"]); +} + +/// The other route to the same statement: a fragment numbered from `$1` and +/// renumbered afterwards. Both paths have to produce the same rows, or one of +/// them is binding a slot out. +#[tokio::test] +async fn a_shifted_fragment_agrees_with_a_pre_numbered_one() { + let Some(pool) = pool("splice_shift").await else { + return; + }; + + let shifted = shift("read_count > $1", placeholder_count(LIST_VOLUMES)); + assert_eq!(shifted, "read_count > $3"); + + let sql = splice(LIST_VOLUMES, &[("where", Some(&shifted))]).expect("splice"); + + let rows = sqlx::query(AssertSqlSafe(sql)) + .bind(10_i64) + .bind(0_i64) + .bind(5_i32) + .fetch_all(&pool) + .await + .expect("the spliced statement must run"); + + let titles: Vec = rows.iter().map(|row| row.get("title")).collect(); + // Ordered by id, since no order_by fragment was supplied. + assert_eq!(titles, ["Dune", "Ulysses"]); +} + +/// The property the whole convention rests on: unspliced, the statement is the +/// statement. If the sentinels did not survive as comments, generated SQL could +/// not carry them. +#[tokio::test] +async fn an_unspliced_statement_runs_as_written() { + let Some(pool) = pool("splice_none").await else { + return; + }; + + let sql = splice(LIST_VOLUMES, &[("where", None), ("order_by", None)]).expect("splice"); + + let rows = sqlx::query(AssertSqlSafe(sql)) + .bind(10_i64) + .bind(1_i64) + .fetch_all(&pool) + .await + .expect("the unspliced statement must run"); + + let titles: Vec = rows.iter().map(|row| row.get("title")).collect(); + assert_eq!(titles, ["Emma", "Ulysses"]); +} + +/// The sentinel is a comment, so the *original* has to run too — that is what +/// makes it safe to put in checked-in SQL that other tools also read. +#[tokio::test] +async fn the_statement_with_its_sentinels_intact_runs() { + let Some(pool) = pool("splice_intact").await else { + return; + }; + + let rows = sqlx::query(AssertSqlSafe(LIST_VOLUMES)) + .bind(10_i64) + .bind(0_i64) + .fetch_all(&pool) + .await + .expect("the statement must run with its sentinels in place"); + + assert_eq!(rows.len(), 3); +}