diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..a84167c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,124 @@ +# CI for pg_plan_filter: build with warnings-as-errors and run the +# regression suite against every supported PostgreSQL major version, plus a +# static-analysis pass and a report-only benchmark. +# +# The test job builds its own throwaway cluster (initdb into /tmp) rather +# than using the Debian-packaged one so that the server, headers, and +# pg_regress all come from the same pgdg major version. +name: CI + +on: + push: + branches: [master] + pull_request: + +# Nothing here needs to write to the repository. +permissions: + contents: read + +jobs: + test: + name: PostgreSQL ${{ matrix.pg }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + pg: [14, 15, 16, 17, 18] + env: + PG_CONFIG: /usr/lib/postgresql/${{ matrix.pg }}/bin/pg_config + steps: + - uses: actions/checkout@v4 + + - name: Install PostgreSQL ${{ matrix.pg }} + run: | + sudo /usr/share/postgresql-common/pgdg/apt.postgresql.org.sh -y + sudo apt-get install -y --no-install-recommends \ + postgresql-${{ matrix.pg }} postgresql-server-dev-${{ matrix.pg }} + + - name: Build (warnings are errors) + run: make PG_CONFIG="$PG_CONFIG" COPT=-Werror + + - name: Install + run: sudo make install PG_CONFIG="$PG_CONFIG" + + - name: Regression tests + run: | + PGBIN=$("$PG_CONFIG" --bindir) + "$PGBIN/initdb" -D /tmp/pgdata -A trust + { + echo "port = 55432" + echo "unix_socket_directories = '/tmp'" + } >> /tmp/pgdata/postgresql.conf + "$PGBIN/pg_ctl" -D /tmp/pgdata -l /tmp/pg.log start + PGHOST=/tmp PGPORT=55432 make installcheck PG_CONFIG="$PG_CONFIG" + + - name: Show regression diffs + if: failure() + run: | + cat regression.diffs 2>/dev/null || true + tail -50 /tmp/pg.log 2>/dev/null || true + + lint: + name: Static analysis + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install cppcheck and PostgreSQL headers + run: | + sudo /usr/share/postgresql-common/pgdg/apt.postgresql.org.sh -y + sudo apt-get install -y --no-install-recommends \ + cppcheck postgresql-server-dev-18 + + - name: cppcheck + # Findings inside the PostgreSQL headers themselves are suppressed; + # only findings in this module's code fail the job. + run: | + cppcheck --std=c99 --quiet --enable=warning,portability \ + --error-exitcode=2 --inline-suppr \ + --suppress='*:*/postgresql/*' \ + -I "$(/usr/lib/postgresql/18/bin/pg_config --includedir-server)" \ + plan_filter.c + + # Report-only: shared CI runners are too noisy to gate on raw benchmark + # numbers, so this job never fails the pipeline. Results land in the job + # summary and are uploaded as an artifact for trend-tracking. + benchmark: + name: Benchmark (report only) + runs-on: ubuntu-latest + continue-on-error: true + steps: + - uses: actions/checkout@v4 + + - name: Install PostgreSQL 18 + run: | + sudo /usr/share/postgresql-common/pgdg/apt.postgresql.org.sh -y + sudo apt-get install -y --no-install-recommends \ + postgresql-18 postgresql-server-dev-18 + + - name: Build and install + run: | + make PG_CONFIG=/usr/lib/postgresql/18/bin/pg_config + sudo make install PG_CONFIG=/usr/lib/postgresql/18/bin/pg_config + + - name: Run benchmark + # pipefail so a bench failure fails this (non-gating) job visibly + # instead of publishing an empty results file. + run: | + set -o pipefail + PGBIN=$(/usr/lib/postgresql/18/bin/pg_config --bindir) \ + ./bench/run.sh | tee bench-results.txt + + - name: Publish summary + run: | + { + echo '### plan_filter benchmark (report-only)' + echo '```' + cat bench-results.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - uses: actions/upload-artifact@v4 + with: + name: bench-results-${{ github.sha }} + path: bench-results.txt diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e0d5365 --- /dev/null +++ b/.gitignore @@ -0,0 +1,11 @@ +# Build products and pg_regress leavings +*.o +*.so +*.dylib +*.bc +.deps/ +results/ +regression.diffs +regression.out +log/ +tmp_check/ diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 0000000..d8715c2 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,72 @@ +# ARCHITECTURE + +`pg_plan_filter` is a single-file PostgreSQL loadable module (not an +extension: no `.control`, no SQL objects) that installs a `planner_hook` +and raises `ERRCODE_STATEMENT_TOO_COMPLEX` when a completed plan's +`total_cost` exceeds `plan_filter.statement_cost_limit`. Decision +rationale lives in `docs/adr/`; this file is the map and the invariants. + +## Map + +| Path | Role | +|---|---| +| `plan_filter.c` | Everything: GUC definitions, hook install, the filter | +| `Makefile` | PGXS `MODULE_big` build; `REGRESS = plan_filter` | +| `sql/` + `expected/` | pg_regress suite (`make installcheck`) | +| `bench/run.sh` | Self-contained pgbench overhead harness (report-only) | +| `.github/workflows/ci.yml` | Matrix CI; see ADR 0003 | +| `docs/adr/` | Decision records; index in its README.md | + +## Invariants + +- **Support window** is exactly the upstream-supported majors (14–18 as + of 2026); a `#error` enforces the floor. Autumn maintenance: bump the + CI matrix, raise the floor when the oldest major EOLs, update the + hard-coded PG-18 references in the lint/benchmark jobs and the README + version sentence (ADR 0001, 0003). +- **Hook discipline**: `limit_func` calls exactly one of + `prev_planner_hook` or `standard_planner`, and checks the cost only + *after* planning completes, on the finished `PlannedStmt`. The error + is thrown, never returned. +- **Both feature GUCs are `PGC_SUSET` on purpose** — an unprivileged user + must not be able to lift an administrator's limit. The regression + suite tests this; do not downgrade the context. +- **`plan_filter.module_loaded`** exists only as a presence signal + (`PGC_BACKEND`, defined at load time); nothing reads it in C. +- **No `_PG_fini`** — PostgreSQL 15 removed library unloading; do not + reintroduce it. +- **Expected output must stay version-independent**: no cost numbers, no + `EXPLAIN` without `COSTS OFF`, no version-varying messages. One + expected file serves every supported major on every platform + (ADR 0002). +- **The suite pins both GUCs immediately after `LOAD`** so it passes on + clusters that preload the module with a limit configured. Keep any new + plannable statement below that pin point out of the file. +- **CI gates on correctness only**; the benchmark job is report-only and + must stay `continue-on-error` (ADR 0003). + +## Landmines + +- The `plan_filter.` GUC prefix is reserved at load on 15+ + (`MarkGUCPrefixReserved`): misspelled `SET plan_filter.x` errors. On + 14 the older call only warns about preexisting placeholders — behavior + differs by major. +- `EXPLAIN` is filtered too (it plans). The documented escape is + `SET LOCAL plan_filter.statement_cost_limit = 0` in a transaction. +- `filter_select_only` exempts by `parse->commandType != CMD_SELECT`; + a SELECT with data-modifying CTEs is still `CMD_SELECT` and still + filtered — SELECT ≠ read-only, as the README warns. + +## Working on this repo + +- Build/test: `make && make install`, then `make installcheck` against a + running server (libpq env vars select it). +- Testing without install rights: add the build directory to a scratch + cluster's `dynamic_library_path`; `LOAD 'plan_filter'` finds the built + library there. +- Full pre-push rehearsal: the CI steps run verbatim in an + `ubuntu:24.04` container using `apt.postgresql.org.sh`; build with + `with_llvm=no` if clang is absent. +- New/changed C code must build warning-free (`COPT=-Werror` is the CI + gate) and follow PostgreSQL backend conventions (tabs, `/* */` + comments, errors via `ereport`). diff --git a/Makefile b/Makefile index 7598a89..4b0df2c 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ MODULE_big = plan_filter OBJS = plan_filter.o $(WIN32RES) PGFILEDESC = "filter statements meeting plan criteria - currently by plan cost" DOCS = $(wildcard doc/*.md) +REGRESS = plan_filter PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) diff --git a/README.md b/README.md index e069956..4872052 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ Plan Filter Module for PostgreSQL ================================= +[![CI](https://github.com/pgexperts/pg_plan_filter/actions/workflows/ci.yml/badge.svg)](https://github.com/pgexperts/pg_plan_filter/actions/workflows/ci.yml) + This loadable module will test statements against specific configured criteria before execution, raising an error if the criteria are violated. This allows administrators to prevent execution of certain queries on @@ -22,18 +24,13 @@ which means do not apply any filter. So a typical pair of settings in the shared_preload_libraries = 'plan_filter' plan_filter.statement_cost_limit = 100000.0 -`ļimit_select_only` limits filtering to SELECT statements only. The default is false. +`filter_select_only` limits filtering to SELECT statements only. The default is false. - plan_filter.limit_select_only = true + plan_filter.filter_select_only = true turns it on. Be aware that SELECT != READONLY, since SELECT statements might also modify data. -If you're using this with a version of PostgreSQL prior to 9.2, you will -need also to have a line like this before the above lines: - - custom_variable_classes = 'plan_filter' - When this module is running with a non-zero `statement_cost_limit`, it will also prevent `EXPLAIN` on expensive queries. The solution would be to `set statement_cost_limit` temporarily to 0 and then run the `EXPLAIN`, @@ -63,8 +60,21 @@ like this: As `pg_plan_filter` is a loadable module rather than an Extension, it cannot be installed using PGXN or other extension-management tools. -This module has been tested on PostgreSQL 9.1.14, 9.3.6 and 9.4.1. It should work on -any version 9.0 or later, but has not necessarily been tested on every release. +This module supports all PostgreSQL major versions with upstream support, +currently 14 through 18, and requires at least 14 to build. Each supported +version is exercised in CI on every pull request and every push to master. + +Testing +------- + +With the module built and installed, and a server from the same build +running, the regression suite can be run with: + + make installcheck + +Standard libpq environment variables (`PGHOST`, `PGPORT`, and so on) select +the server to test against. The test session loads the module with `LOAD`, +so nothing needs to be added to `shared_preload_libraries` first. Warnings -------- diff --git a/bench/run.sh b/bench/run.sh new file mode 100755 index 0000000..2126407 --- /dev/null +++ b/bench/run.sh @@ -0,0 +1,64 @@ +#!/bin/sh +# Measures the planner-hook overhead of plan_filter: pgbench select-only +# throughput with the module absent vs. loaded with a cost limit high enough +# that the check runs but never fires. Numbers from shared CI runners are +# too noisy to gate on, so this is for trend-tracking only. +# +# Environment: +# PGBIN bin directory of the PostgreSQL to use (default: pg_config) +# BENCH_SECONDS per-configuration pgbench duration (default: 10) +# PLAN_FILTER_DIR if set, added to dynamic_library_path so an uninstalled +# build directory can be benchmarked +set -eu + +PGBIN=${PGBIN:-$(pg_config --bindir)} +DATADIR=$(mktemp -d) +export PGHOST="$DATADIR" PGPORT=55433 PGDATABASE=postgres + +# On failure, dump the server log before it is removed with the datadir. +# The INT/TERM traps exist because POSIX shells do not run the EXIT trap +# when killed by an unhandled signal (Ctrl-C, CI job cancellation). +cleanup() { + status=$? + if [ "$status" -ne 0 ] && [ -f "$DATADIR/log" ]; then + echo "--- server log tail ---" >&2 + tail -20 "$DATADIR/log" >&2 + fi + "$PGBIN/pg_ctl" -D "$DATADIR/data" stop -m immediate >/dev/null 2>&1 || true + rm -rf "$DATADIR" + exit "$status" +} +trap cleanup EXIT +trap 'exit 130' INT TERM + +"$PGBIN/initdb" -D "$DATADIR/data" -A trust >/dev/null +{ + echo "port = $PGPORT" + echo "listen_addresses = ''" + echo "unix_socket_directories = '$DATADIR'" + echo "fsync = off" + if [ -n "${PLAN_FILTER_DIR:-}" ]; then + echo "dynamic_library_path = '$PLAN_FILTER_DIR:\$libdir'" + fi +} >> "$DATADIR/data/postgresql.conf" +"$PGBIN/pg_ctl" -D "$DATADIR/data" -l "$DATADIR/log" start >/dev/null + +"$PGBIN/createdb" bench +"$PGBIN/pgbench" -i -s 1 -q bench >/dev/null + +# Capture pgbench's output in a variable so its exit status is not masked +# by a pipeline; fail if the expected tps line never appears. +run() { + out=$("$PGBIN/pgbench" -S -n -c 4 -j 4 -T "${BENCH_SECONDS:-10}" bench) + printf '%s\n' "$out" | + awk -v label="$1" '/^tps/ {printf "%-28s %s tps\n", label, $3; found = 1; exit} + END {if (!found) exit 1}' +} + +run "baseline (no module)" + +"$PGBIN/psql" -q -d bench \ + -c "ALTER DATABASE bench SET session_preload_libraries = 'plan_filter'" \ + -c "ALTER DATABASE bench SET plan_filter.statement_cost_limit = 1e9" + +run "plan_filter (limit unhit)" diff --git a/docs/adr/0001-support-upstream-supported-majors-only.md b/docs/adr/0001-support-upstream-supported-majors-only.md new file mode 100644 index 0000000..a2bdd31 --- /dev/null +++ b/docs/adr/0001-support-upstream-supported-majors-only.md @@ -0,0 +1,58 @@ +--- +id: 0001 +title: Support only upstream-supported PostgreSQL majors (currently 14-18) +date: 2026-07-21 +status: Accepted +summary: The module targets the PostgreSQL majors with upstream support, enforces a hard floor of 14 at compile time, and drops all pre-13 compatibility scaffolding. +--- + +# 0001. Support only upstream-supported PostgreSQL majors (currently 14-18) + +## Context + +The module was written for PostgreSQL 9.x and last touched for +compatibility when PostgreSQL 13 changed the planner-hook signature (it +gained `const char *query_string`). The source carried `#if`-macro +scaffolding (`PLANNER_HOOK_PARAMS` / `PLANNER_HOOK_ARGS`) so one body +could compile against both pre-13 and 13+ signatures, plus a `_PG_fini` +unload callback. As of July 2026 the upstream-supported majors are 14 +through 18; everything the macros existed for is end-of-life. PostgreSQL +15 removed library unloading entirely (so `_PG_fini` is dead code) and +added `MarkGUCPrefixReserved`, which turns a misspelled +`plan_filter.*` setting from a silently-ignored placeholder into an error +— a meaningful safety property for a module whose whole job is being a +guard rail. + +## Decision + +Track the upstream support window and nothing older. Concretely: a +compile-time `#error` below `PG_VERSION_NUM 140000`; the 13+ planner-hook +signature written directly with the compatibility macros deleted; +`_PG_fini` removed; the `plan_filter.` GUC prefix reserved +(`MarkGUCPrefixReserved` on 15+, `EmitWarningsOnPlaceholders` on 14); and +`PG_MODULE_MAGIC_EXT` with a module name and version on 18+, falling back +to plain `PG_MODULE_MAGIC` earlier. + +## Alternatives considered + +- **Keep the compatibility macros and let pre-13 keep building** — the + macros cost little, but untested-is-unsupported: nothing older than 14 + is exercised by CI, and advertising support that is never verified is + how version bit-rot went unnoticed here for years in the first place. +- **Set the floor at 13** — the code is identical under 13, but 13 went + end-of-life in November 2025 and would be one more untested + configuration; users on 13 can build from an earlier checkout. +- **Version-sniff with `#ifdef` around every changed API instead of a + floor** — appropriate for extensions that must span many majors; + pointless here since the 14-18 API surface this module touches is + uniform except for the two guarded calls noted above. + +## Consequences + +The source is a single straight-line file with two small version guards +(GUC-prefix reservation, module magic). Each autumn's new major and +EOL is a policy update, not an archaeology project: bump the CI matrix, +raise the floor when the oldest supported major moves. Users on EOL +versions must build from an older commit — a deliberate cost. The +GUC-prefix reservation is a minor behavior change on 15+: `SET +plan_filter.typo = ...` now errors instead of being silently accepted. diff --git a/docs/adr/0002-behavioral-pg-regress-suite.md b/docs/adr/0002-behavioral-pg-regress-suite.md new file mode 100644 index 0000000..ad863c2 --- /dev/null +++ b/docs/adr/0002-behavioral-pg-regress-suite.md @@ -0,0 +1,65 @@ +--- +id: 0002 +title: Behavioral pg_regress suite with version-independent expected output +date: 2026-07-21 +status: Accepted +summary: Testing uses a standard pg_regress suite whose output contains no cost numbers or plans, so one expected file serves every supported major on every platform. +--- + +# 0002. Behavioral pg_regress suite with version-independent expected output + +## Context + +The only test was `testlimit.sql`, a manual psql script: it built a +100-million-row table, reconnected as a superuser test role with `\c`, +and had no expected output — a human eyeballed the results. Nothing +about it could run under CI, and it had no way to fail automatically. +The obvious hazard in testing a cost-based filter is that cost estimates +differ across majors, platforms, and planner changes, so naive expected +files (anything containing `EXPLAIN` costs) would need per-version +variants and constant re-blessing. + +## Decision + +Test through the standard PGXS `REGRESS` machinery (`sql/plan_filter.sql` ++ `expected/plan_filter.out`, run via `make installcheck`) and keep every +assertion behavioral: statements either succeed or raise the module's +error. Cost numbers never appear in the output — the one `EXPLAIN` in +the suite uses `COSTS OFF` and exists only to prove EXPLAIN itself is +filtered. The threshold trick that makes this work: a 10,000-row table +with `statement_cost_limit = 1` sits so far from the boundary on both +sides (the filtered statements plan at a total cost around 190, trivial +statements around 0.01) that no realistic planner change can flip an +assertion. One expected file covers all supported majors on all +platforms. The suite pins both GUCs to their defaults immediately after +`LOAD`, so it also passes against a cluster whose configuration preloads +the module with a limit already set. It exercises: limit off, limit +exceeded, cheap statement passing, EXPLAIN blocked, `SET LOCAL` escape +hatch and its expiry, `filter_select_only` in both positions, and the +superuser-only (`PGC_SUSET`) enforcement of both GUCs. `testlimit.sql` +is deleted, not kept alongside. + +## Alternatives considered + +- **Keep `testlimit.sql` as a manual supplement** — an unrunnable + near-duplicate of the real suite would only drift; the regress suite + covers everything it did except sheer table size, which tested the + planner's arithmetic rather than this module's logic. +- **Per-version expected files (`plan_filter_1.out`, ...)** — pg_regress + supports alternates, but they are a standing maintenance tax and this + design makes them unnecessary by construction. +- **TAP tests (`PROVE_TESTS`)** — strictly more powerful (could test + `shared_preload_libraries` mode), but needs a Perl toolchain and + `PostgreSQL::Test` modules that pgdg packages don't ship cleanly for + extensions; pg_regress is the conventional floor for a module this + small. + +## Consequences + +`make installcheck` is a real pass/fail gate usable locally and in CI, +against any running server of any supported version. The suite runs in +about a second. What is *not* covered: `shared_preload_libraries` +loading (the suite uses per-session `LOAD`; the code path difference is +negligible for a planner hook) and the interaction of reserved GUC +prefixes with preexisting placeholder settings. Expected-file updates +are only ever needed if the module's own messages change. diff --git a/docs/adr/0003-github-actions-ci-pgdg-matrix.md b/docs/adr/0003-github-actions-ci-pgdg-matrix.md new file mode 100644 index 0000000..91d1ffb --- /dev/null +++ b/docs/adr/0003-github-actions-ci-pgdg-matrix.md @@ -0,0 +1,69 @@ +--- +id: 0003 +title: GitHub Actions CI on the pgdg apt matrix, with report-only benchmarks +date: 2026-07-21 +status: Accepted +summary: CI builds with -Werror and runs installcheck against pgdg packages for every supported major, runs cppcheck as lint, and publishes pgbench numbers as a non-gating report. +--- + +# 0003. GitHub Actions CI on the pgdg apt matrix, with report-only benchmarks + +## Context + +The project had no CI; version compatibility was discovered by user bug +report (the PG 13 fix arrived that way). A cost-filter module is pure +hook-and-GUC code, so the entire risk surface is "does it still compile +and behave against major N" — exactly what a version matrix answers. +Per-project convention, correctness must gate the pipeline while +benchmark numbers from shared runners must not, because noisy-neighbor +CI hardware produces 2x swings on identical code. + +## Decision + +One GitHub Actions workflow with three job groups, running on push to +master and on pull requests: + +- **test** (gating): matrix over PostgreSQL 14-18 from apt.postgresql.org + on `ubuntu-latest`, via the `apt.postgresql.org.sh` helper that ships + in the runner image's `postgresql-common`. Each leg builds with + `COPT=-Werror`, installs, `initdb`s its own throwaway cluster as the + runner user (socket in `/tmp`, port 55432), and runs `make + installcheck`. On failure it dumps `regression.diffs` and the server + log. +- **lint** (gating): `cppcheck --enable=warning,portability` against the + PG 18 server headers. +- **benchmark** (report-only, `continue-on-error`): pgbench select-only + throughput, module absent vs. loaded with an unreached limit, on PG 18; + results go to the job summary and an uploaded artifact for + trend-tracking across commits. + +## Alternatives considered + +- **Use the Debian-packaged auto-created cluster** (`pg_ctlcluster`) — + it runs as the `postgres` OS user, so `make installcheck` from the + runner-owned checkout hits permission problems writing `results/`, and + its socket lives in a directory owned by the `postgres` user that the + runner user cannot create sockets in. A self-initdb'd cluster + owned by the runner user avoids the whole class of problem and pins + server and headers to the same pgdg major. +- **Docker `postgres:N` images as services** — the official images ship + no server headers or pgxs; building inside a container per version + means maintaining Dockerfiles that duplicate what pgdg packages already + provide. +- **Third-party setup actions** (`ankane/setup-postgres` and kin) — one + more supply-chain dependency to pin and audit, replacing two lines of + apt. +- **Gating on benchmark numbers** — rejected outright: shared-runner + variance exceeds any plausible regression signal for a module whose + hot path is one float comparison. + +## Consequences + +Every pull request and push to master now proves build cleanliness +(warnings are errors) and behavior on all five supported majors in a few +minutes. Autumn maintenance is editing the matrix list (add the new +major, drop the EOL one) and the hard-coded PG-18 version references in +the lint and benchmark jobs. The benchmark provides a paper trail rather than protection — a +real perf regression must be caught by a human reading the trend. The +workflow depends on pgdg publishing packages for new majors promptly, +which it reliably does. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..906e65b --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,9 @@ +# Architecture Decision Records + + + +| ID | Title | Status | Date | Summary | +|---:|-------|--------|------|---------| +| [0001](0001-support-upstream-supported-majors-only.md) | Support only upstream-supported PostgreSQL majors (currently 14-18) | Accepted | 2026-07-21 | The module targets the PostgreSQL majors with upstream support, enforces a hard floor of 14 at compile time, and drops all pre-13 compatibility scaffolding. | +| [0002](0002-behavioral-pg-regress-suite.md) | Behavioral pg_regress suite with version-independent expected output | Accepted | 2026-07-21 | Testing uses a standard pg_regress suite whose output contains no cost numbers or plans, so one expected file serves every supported major on every platform. | +| [0003](0003-github-actions-ci-pgdg-matrix.md) | GitHub Actions CI on the pgdg apt matrix, with report-only benchmarks | Accepted | 2026-07-21 | CI builds with -Werror and runs installcheck against pgdg packages for every supported major, runs cppcheck as lint, and publishes pgbench numbers as a non-gating report. | diff --git a/expected/plan_filter.out b/expected/plan_filter.out new file mode 100644 index 0000000..b3ee147 --- /dev/null +++ b/expected/plan_filter.out @@ -0,0 +1,83 @@ +-- +-- Behavioral tests for plan_filter. Cost estimates never appear in the +-- output (no EXPLAIN with costs on), so the expected file is stable across +-- PostgreSQL versions and platforms. +-- +LOAD 'plan_filter'; +SHOW plan_filter.module_loaded; + plan_filter.module_loaded +--------------------------- + on +(1 row) + +-- Pin both GUCs before doing anything plannable, so the suite also passes +-- on a cluster whose configuration preloads the module with a limit set. +SET plan_filter.statement_cost_limit = 0; +SET plan_filter.filter_select_only = false; +CREATE TABLE plan_filter_test AS + SELECT g AS x, g % 100 AS y FROM generate_series(1, 10000) g; +ANALYZE plan_filter_test; +-- a limit of zero means no filtering +SET plan_filter.statement_cost_limit = 0; +SELECT count(*) FROM plan_filter_test; + count +------- + 10000 +(1 row) + +-- a tiny limit blocks the sequential scan +SET plan_filter.statement_cost_limit = 1; +SELECT count(*) FROM plan_filter_test; +ERROR: plan cost limit exceeded +HINT: The plan for your query shows that it would probably have an excessive run time. This may be due to a logic error in the SQL, or it may be just a very costly query. Rewrite your query or increase the configuration parameter "plan_filter.statement_cost_limit". +-- cheap statements still pass +SELECT 1 AS cheap; + cheap +------- + 1 +(1 row) + +-- EXPLAIN plans the query, so it is blocked as well +EXPLAIN (COSTS OFF) SELECT count(*) FROM plan_filter_test; +ERROR: plan cost limit exceeded +HINT: The plan for your query shows that it would probably have an excessive run time. This may be due to a logic error in the SQL, or it may be just a very costly query. Rewrite your query or increase the configuration parameter "plan_filter.statement_cost_limit". +-- SET LOCAL provides a temporary escape hatch +BEGIN; +SET LOCAL plan_filter.statement_cost_limit = 0; +SELECT count(*) FROM plan_filter_test; + count +------- + 10000 +(1 row) + +COMMIT; +-- and the limit comes back after commit +SELECT count(*) FROM plan_filter_test; +ERROR: plan cost limit exceeded +HINT: The plan for your query shows that it would probably have an excessive run time. This may be due to a logic error in the SQL, or it may be just a very costly query. Rewrite your query or increase the configuration parameter "plan_filter.statement_cost_limit". +-- filter_select_only exempts non-SELECT statements from the limit +SET plan_filter.filter_select_only = true; +BEGIN; +UPDATE plan_filter_test SET y = y + 1; +ROLLBACK; +-- but SELECT is still filtered +SELECT count(*) FROM plan_filter_test; +ERROR: plan cost limit exceeded +HINT: The plan for your query shows that it would probably have an excessive run time. This may be due to a logic error in the SQL, or it may be just a very costly query. Rewrite your query or increase the configuration parameter "plan_filter.statement_cost_limit". +-- with filter_select_only off, expensive DML is blocked too +SET plan_filter.filter_select_only = false; +BEGIN; +UPDATE plan_filter_test SET y = y + 1; +ERROR: plan cost limit exceeded +HINT: The plan for your query shows that it would probably have an excessive run time. This may be due to a logic error in the SQL, or it may be just a very costly query. Rewrite your query or increase the configuration parameter "plan_filter.statement_cost_limit". +ROLLBACK; +-- both GUCs are superuser-only: an unprivileged role cannot lift the limit +CREATE ROLE regress_plan_filter_user; +SET ROLE regress_plan_filter_user; +SET plan_filter.statement_cost_limit = 0; +ERROR: permission denied to set parameter "plan_filter.statement_cost_limit" +SET plan_filter.filter_select_only = true; +ERROR: permission denied to set parameter "plan_filter.filter_select_only" +RESET ROLE; +DROP ROLE regress_plan_filter_user; +DROP TABLE plan_filter_test; diff --git a/plan_filter.c b/plan_filter.c index b7a7fd9..cde347b 100644 --- a/plan_filter.c +++ b/plan_filter.c @@ -21,30 +21,19 @@ #include "optimizer/planner.h" #include "utils/guc.h" -#define PG13_GTE (PG_VERSION_NUM >= 130000) - -#if PG13_GTE -#define PLANNER_HOOK_PARAMS \ - Query *parse, const char *queryString \ -, int cursorOptions, ParamListInfo boundParams -#else -#define PLANNER_HOOK_PARAMS \ - Query *parse \ -, int cursorOptions, ParamListInfo boundParams +#if PG_VERSION_NUM < 140000 +#error "pg_plan_filter requires PostgreSQL 14 or later" #endif -#if PG13_GTE -#define PLANNER_HOOK_ARGS \ - parse, queryString \ -, cursorOptions, boundParams +/* + * PG_MODULE_MAGIC_EXT (PostgreSQL 18+) lets the server report the module's + * name and version; older releases only have the plain magic block. + */ +#ifdef PG_MODULE_MAGIC_EXT +PG_MODULE_MAGIC_EXT(.name = "plan_filter", .version = "1.0.0"); #else -#define PLANNER_HOOK_ARGS \ - parse \ -, cursorOptions, boundParams -#endif - - PG_MODULE_MAGIC; +#endif static double statement_cost_limit = 0.0; @@ -54,13 +43,18 @@ static bool filter_select_only = false; static planner_hook_type prev_planner_hook = NULL; -static PlannedStmt *limit_func(PLANNER_HOOK_PARAMS); +static PlannedStmt *limit_func(Query *parse, const char *query_string, + int cursorOptions, + ParamListInfo boundParams); void _PG_init(void); -void _PG_fini(void); /* - * Module load callback + * Module load callback. + * + * There is deliberately no _PG_fini: PostgreSQL 15 removed the ability to + * unload a shared library, and it was not reachable in practice before + * that, so the hook stays installed for the life of the backend. */ void _PG_init(void) @@ -91,7 +85,7 @@ _PG_init(void) NULL, NULL); - /* Define custom GUC variable. */ + /* Define custom GUC variable. */ DefineCustomBoolVariable("plan_filter.filter_select_only", "Limit the filter to SELECT queries " "only.", @@ -104,36 +98,38 @@ _PG_init(void) NULL, NULL); + /* + * Reserve the "plan_filter." prefix so that a misspelled setting is an + * error rather than a silently-ignored placeholder that would leave the + * filter turned off. PostgreSQL 14 only has the older call, which just + * warns about existing placeholders at load time. + */ +#if PG_VERSION_NUM >= 150000 + MarkGUCPrefixReserved("plan_filter"); +#else + EmitWarningsOnPlaceholders("plan_filter"); +#endif + /* install the hook */ prev_planner_hook = planner_hook; planner_hook = limit_func; - -} - -/* - * Module unload callback - */ -void -_PG_fini(void) -{ - /* Uninstall hook. */ - planner_hook = prev_planner_hook; - /* reset loaded var */ - module_loaded = false; } static PlannedStmt * -limit_func(PLANNER_HOOK_PARAMS) +limit_func(Query *parse, const char *query_string, int cursorOptions, + ParamListInfo boundParams) { PlannedStmt *result; /* this way we can daisy chain planner hooks if necessary */ if (prev_planner_hook != NULL) - result = (*prev_planner_hook) (PLANNER_HOOK_ARGS); + result = (*prev_planner_hook) (parse, query_string, cursorOptions, + boundParams); else - result = standard_planner(PLANNER_HOOK_ARGS); + result = standard_planner(parse, query_string, cursorOptions, + boundParams); - if(filter_select_only && parse->commandType != CMD_SELECT) + if (filter_select_only && parse->commandType != CMD_SELECT) return result; if (statement_cost_limit > 0.0 && @@ -141,12 +137,12 @@ limit_func(PLANNER_HOOK_PARAMS) ereport(ERROR, (errcode(ERRCODE_STATEMENT_TOO_COMPLEX), errmsg("plan cost limit exceeded"), - errhint("The plan for your query shows that it would probably " - "have an excessive run time. This may be due to a " - "logic error in the SQL, or it maybe just a very " - "costly query. Rewrite your query or increase the " - "configuration parameter " - "\"plan_filter.statement_cost_limit\"."))); + errhint("The plan for your query shows that it would probably " + "have an excessive run time. This may be due to a " + "logic error in the SQL, or it may be just a very " + "costly query. Rewrite your query or increase the " + "configuration parameter " + "\"plan_filter.statement_cost_limit\"."))); return result; } diff --git a/sql/plan_filter.sql b/sql/plan_filter.sql new file mode 100644 index 0000000..5e6c442 --- /dev/null +++ b/sql/plan_filter.sql @@ -0,0 +1,64 @@ +-- +-- Behavioral tests for plan_filter. Cost estimates never appear in the +-- output (no EXPLAIN with costs on), so the expected file is stable across +-- PostgreSQL versions and platforms. +-- +LOAD 'plan_filter'; +SHOW plan_filter.module_loaded; + +-- Pin both GUCs before doing anything plannable, so the suite also passes +-- on a cluster whose configuration preloads the module with a limit set. +SET plan_filter.statement_cost_limit = 0; +SET plan_filter.filter_select_only = false; + +CREATE TABLE plan_filter_test AS + SELECT g AS x, g % 100 AS y FROM generate_series(1, 10000) g; +ANALYZE plan_filter_test; + +-- a limit of zero means no filtering +SET plan_filter.statement_cost_limit = 0; +SELECT count(*) FROM plan_filter_test; + +-- a tiny limit blocks the sequential scan +SET plan_filter.statement_cost_limit = 1; +SELECT count(*) FROM plan_filter_test; + +-- cheap statements still pass +SELECT 1 AS cheap; + +-- EXPLAIN plans the query, so it is blocked as well +EXPLAIN (COSTS OFF) SELECT count(*) FROM plan_filter_test; + +-- SET LOCAL provides a temporary escape hatch +BEGIN; +SET LOCAL plan_filter.statement_cost_limit = 0; +SELECT count(*) FROM plan_filter_test; +COMMIT; + +-- and the limit comes back after commit +SELECT count(*) FROM plan_filter_test; + +-- filter_select_only exempts non-SELECT statements from the limit +SET plan_filter.filter_select_only = true; +BEGIN; +UPDATE plan_filter_test SET y = y + 1; +ROLLBACK; + +-- but SELECT is still filtered +SELECT count(*) FROM plan_filter_test; + +-- with filter_select_only off, expensive DML is blocked too +SET plan_filter.filter_select_only = false; +BEGIN; +UPDATE plan_filter_test SET y = y + 1; +ROLLBACK; + +-- both GUCs are superuser-only: an unprivileged role cannot lift the limit +CREATE ROLE regress_plan_filter_user; +SET ROLE regress_plan_filter_user; +SET plan_filter.statement_cost_limit = 0; +SET plan_filter.filter_select_only = true; +RESET ROLE; +DROP ROLE regress_plan_filter_user; + +DROP TABLE plan_filter_test; diff --git a/testlimit.sql b/testlimit.sql deleted file mode 100644 index 97cffaa..0000000 --- a/testlimit.sql +++ /dev/null @@ -1,72 +0,0 @@ - --- set up a big table if it's not already there, we're using 100m rows here - -do $$ - -begin - - perform 1 from pg_class where relname = 'bigtest'; - - if not found - then - - create table bigtest as - select x , y - from generate_series(1,10000) x, - generate_series(10001,20000) y; - alter table bigtest add primary key (x,y); - analyse bigtest; - - end if; - -end; - -$$; - -select coalesce((select setting::boolean from pg_settings where name = 'plan_filter.module_loaded'),false) as have_plan_filter_module; - -LOAD 'plan_filter'; -SET plan_filter.statement_cost_limit = 100000; - -select coalesce((select setting::boolean from pg_settings where name = 'plan_filter.module_loaded'),false) as have_plan_filter_module; - --- should fail - -explain select * from bigtest; - -select * from bigtest; - - --- temporary override -begin; -set local plan_filter.statement_cost_limit = 0; -explain select * from bigtest; -commit; - --- back to limited - -explain select * from bigtest; - --- override by user --- test probably runs best with trust authentication - -drop role if exists testuser; - -create user testuser superuser; -- so they can run LOAD -alter user testuser set plan_filter.statement_cost_limit = 100000; - -\c - testuser - -LOAD 'plan_filter'; - --- should fail without them setting a limit in this session - -explain select * from bigtest; - -select * from bigtest; - --- override the limit - -SET plan_filter.statement_cost_limit = 0; - -explain select * from bigtest;