Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Benchmark

permissions: {}

on:
pull_request:

concurrency:
cancel-in-progress: true
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}

jobs:
benchmark:
name: Query performance
runs-on: ubuntu-latest
permissions:
pull-requests: write
services:
postgres:
image: postgis/postgis:18-3.6
env:
POSTGRES_DB: ci_database
POSTGRES_PASSWORD: postgres
POSTGRES_USER: ci
options: >-
--health-cmd "pg_isready -U ci"
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432
timeout-minutes: 15
steps:
- name: Check out PR branch
uses: actions/checkout@v4

- name: Check out base branch
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.base.ref }}
clean: false
path: base

- name: Create role secrets
run: |
sudo mkdir -p /run/secrets
for role in grafana postgraphile vibetype zammad; do
echo "$role" | sudo tee "/run/secrets/postgres_role_service_${role}_username" > /dev/null
echo "placeholder" | sudo tee "/run/secrets/postgres_role_service_${role}_password" > /dev/null
done

- name: Install Sqitch
run: sudo apt-get update && sudo apt-get install --no-install-recommends -y jq=1.7.1-3ubuntu0.24.04.1 sqitch=1.4.1-1

- name: Deploy base migrations
run: sqitch --chdir base/src deploy --target db:pg://ci:postgres@localhost/ci_database

- name: Run base benchmarks
run: ./test/benchmark/run.sh /tmp/benchmark_base.json "pg://ci:postgres@localhost/ci_database"

- name: Revert base migrations
run: sqitch --chdir base/src revert --target db:pg://ci:postgres@localhost/ci_database -y

- name: Deploy PR migrations
run: sqitch --chdir src deploy --target db:pg://ci:postgres@localhost/ci_database

- name: Run PR benchmarks
run: ./test/benchmark/run.sh /tmp/benchmark_pr.json "pg://ci:postgres@localhost/ci_database"

- name: Generate comparison
run: >-
./test/benchmark/compare.sh
/tmp/benchmark_base.json
/tmp/benchmark_pr.json
/tmp/benchmark_comment.md
"${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"

- name: Find comment
uses: peter-evans/find-comment@v3
id: fc
with:
body-includes: Database Query Performance
comment-author: 'github-actions[bot]'
issue-number: ${{ github.event.pull_request.number }}

- name: Post or update PR comment
uses: peter-evans/create-or-update-comment@v4
with:
body-path: /tmp/benchmark_comment.md
comment-id: ${{ steps.fc.outputs.comment-id }}
edit-mode: replace
issue-number: ${{ github.event.pull_request.number }}
10 changes: 1 addition & 9 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
{
"editor.defaultFormatter": "esbenp.prettier-vscode",
"shellcheck.customArgs": ["-x"],
"[json]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[jsonc]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[yaml]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
}
}
52 changes: 52 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
applyTo: '**'
---
# Project Instructions
This is a PostgreSQL migration project using [Sqitch](https://sqitch.org/) for schema management. It defines one of many services of `vibetype`, an event community platform. The SQL schema applied by this service is exposed as a GraphQL API by the `postgraphile` service.

## Migration Structure

- Each migration consists of three files in `src/`: `deploy/<name>.sql`, `revert/<name>.sql`, and `verify/<name>.sql`
- Migrations are wrapped in transactions (`BEGIN;`/`COMMIT;`)
- All migrations are tracked in `src/sqitch.plan`

## API Design

- The PostgreSQL schema is exposed as a GraphQL API via PostGraphile v5
- The GraphQL API exposed should have a simple and consistent surface, especially when it comes to naming
- Prefer CRUD behavior and constraints over custom functions, to keep the schema simple and maintainable
- The `vibetype.jwt` composite type defines JWT payload fields; PostGraphile uses it for JWT signing
- Error codes consist of 5 capitalized letters starting with a `VT` prefix: e.g. `VTPLL` for "password length low"

## Security

- `ENABLE ROW LEVEL SECURITY` (RLS) policies to enforce access control on tables
- Grant `EXECUTE` on functions to the appropriate roles only
- Ensure that PostGraphile Smart Comments properly control API exposure
- Prefer `security invoker`; only use `security definer` when necessary
- Use the `vibetype_private` schema for internal logic and especially protected content that should never be exposed directly

## Performance

- Performance is key: always consider query planner optimizations when writing SQL
- Prefer SQL over PL/pgSQL, except where it doesn't make sense
- Foreign keys must have corresponding indexes

## Testing

- Ensure SQL logic is always covered by tests
- Use the test framework in `test/` rather than `src/verify/` scripts

## Workflow

1. Edit SQL files, keeping the plan in sync
2. Ensure correctness of security and permissions
3. Ensure proper test coverage
4. Run `npm run test:update` to build the Docker test image, deploy all migrations, run test SQL, revert, and update schema fixture files

## General instructions

- Code style
- Sort any elements (imports, object properties, functions, ...), e.g. alphabetically, except when it doesn't make sense.
- Agents
- After making changes to the codebase, ensure AGENTS.md is in sync with your knowledge of the project.
43 changes: 43 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,46 @@
## [11.0.0-beta.4](https://github.com/maevsi/sqitch/compare/11.0.0-beta.3...11.0.0-beta.4) (2026-04-03)

### Bug Fixes

* schedule release ([859e714](https://github.com/maevsi/sqitch/commit/859e71419aa1a5a2b4009db2ae37cf240085c254))
* schedule release ([052f309](https://github.com/maevsi/sqitch/commit/052f30972fd1d6e30c27762dfd4de160b0e6e996))

## [11.0.0-beta.3](https://github.com/maevsi/sqitch/compare/11.0.0-beta.2...11.0.0-beta.3) (2026-03-09)

### Performance Improvements

* inline functions ([8ea356a](https://github.com/maevsi/sqitch/commit/8ea356a871eda41402e054cf2987954ce4dafcdc))

## [11.0.0-beta.2](https://github.com/maevsi/sqitch/compare/11.0.0-beta.1...11.0.0-beta.2) (2026-03-02)

### ⚠ BREAKING CHANGES

* **jwt:** rework mutation

### Features

* **jwt:** rework mutation ([d40493c](https://github.com/maevsi/sqitch/commit/d40493c6e19a26b4f60ec0e3d4ce3d5d7caa5236))

### Bug Fixes

* schedule release ([4ae70d0](https://github.com/maevsi/sqitch/commit/4ae70d05e387717a45217d2a241d14ca90529e6c))
* schedule release ([31be85a](https://github.com/maevsi/sqitch/commit/31be85aaa1e5ba9c69b4ac02d577ce775e345b96))
* schedule release ([6e6e818](https://github.com/maevsi/sqitch/commit/6e6e818c4818518f7309a6a3bea13d1c79f0f6fb))

## [11.0.0-beta.1](https://github.com/maevsi/sqitch/compare/10.0.3...11.0.0-beta.1) (2026-02-20)

### ⚠ BREAKING CHANGES

* **postgraphile:** migrate to v5

### Features

* **postgraphile:** migrate to v5 ([a16af50](https://github.com/maevsi/sqitch/commit/a16af50f84f53916eef5080835bb54f2d95cdfc7))

### Performance Improvements

* **index:** add missing ([9e92140](https://github.com/maevsi/sqitch/commit/9e921401d0b3221090056dc4818463971f570d08))

## [10.0.8](https://github.com/maevsi/sqitch/compare/10.0.7...10.0.8) (2026-03-28)

### Bug Fixes
Expand Down
16 changes: 15 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ COPY ./src ./

###########################
# sqitch is not available for alpine linux as of 2025-11-20 (https://github.com/sqitchers/sqitch/issues/351#issuecomment-614153859)
FROM postgis/postgis:18-3.6 AS test-build
FROM postgis/postgis:18-3.6 AS postgres-base

ENV POSTGRES_DB=ci_database
ENV POSTGRES_PASSWORD_FILE=/run/secrets/postgres_password
Expand All @@ -34,6 +34,7 @@ WORKDIR /srv/app

RUN apt-get update \
&& apt-get install --no-install-recommends -y \
jq \
sqitch=1.5.2-1 \
&& mkdir -p /run/secrets \
&& echo "grafana" > /run/secrets/postgres_role_service_grafana_username \
Expand All @@ -51,6 +52,10 @@ RUN apt-get update \
COPY ./src ./src
COPY ./test ./test


##############################
FROM postgres-base AS test-build

RUN docker-entrypoint.sh postgres & \
while ! pg_isready --host localhost --username ci --port 5432; do sleep 1; done \
&& sqitch --chdir src deploy --target db:pg://ci:postgres@/ci_database \
Expand All @@ -60,6 +65,15 @@ RUN docker-entrypoint.sh postgres & \
--variable TEST_DIRECTORY=./test/logic --variable ON_ERROR_STOP=on \
&& sqitch --chdir src revert --target db:pg://ci:postgres@/ci_database


##############################
FROM postgres-base AS benchmark

RUN docker-entrypoint.sh postgres & \
while ! pg_isready --host localhost --username ci --port 5432; do sleep 1; done \
&& sqitch --chdir src deploy --target db:pg://ci:postgres@/ci_database \
&& ./test/benchmark/run.sh benchmark_results.json "pg://ci:postgres@localhost/ci_database"

##############################
FROM test-build AS test

Expand Down
84 changes: 84 additions & 0 deletions docs/onboarding/database/row_level_security.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,87 @@ Suppose the function was defined as `SECURITY INVOKER`; this would imply the eva

A developer should be careful with `SECURITY DEFINER` functions.
As they bypass RLS security, every security check that would otherwise be handled by policies must be explicitly implemented in the function body.

## Performance considerations for policies

### Avoid wrapping policies in SECURITY DEFINER functions

RLS policy conditions are evaluated per row.
When the entire policy logic is wrapped in a `SECURITY DEFINER` function that takes a row parameter, PostgreSQL treats the function as a black box and evaluates it independently for every row.
Any helper functions called *inside* that wrapper are also re-evaluated per row, even if they return the same result regardless of the row.

For example, a policy like `USING (vibetype_private.event_policy_select(event.*))` that internally calls `events_invited()` (which takes ~18 ms) will take `18 ms × number_of_rows` to evaluate — resulting in **765 ms for just 100 events**.

The preferred approach is to **inline the policy logic** directly in the `USING` clause.
When helper functions appear directly in inline policy expressions, PostgreSQL's optimizer can hoist them into *SubPlans* that are computed once and reused across all rows.

### Use helper functions returning arrays with the unnest pattern

Helper functions like `vibetype_private.account_block_ids()` and `vibetype_private.events_invited()` return `UUID[]` arrays.
Two patterns are used for block checks, depending on context:

#### In policies: NOT EXISTS + unnest (direct function call)

When a block check appears directly in a policy `USING` clause, use `NOT EXISTS` + `unnest()`.
PostgreSQL creates a *hashed SubPlan* that is computed once and probed per row:

```sql
-- Hashed SubPlan: function called once, hash built once, O(1) per-row probe
NOT EXISTS (
SELECT 1 FROM unnest(vibetype_private.account_block_ids()) AS b
WHERE b = event.created_by
)
```

This pattern is inherently NULL-safe: when a column is NULL, `b = NULL` never matches, so `NOT EXISTS` correctly returns TRUE. No `IS NULL OR` guard is needed.

#### In helper functions: _blocked CTE + ANY (when 2+ block checks are needed)

When a SECURITY DEFINER helper function needs to check block status against multiple columns,
use a `_blocked` CTE that stores the UUID array in a single row, cross-joined into the query:

```sql
WITH _blocked AS (
SELECT vibetype_private.account_block_ids() AS ids
)
SELECT ...
FROM vibetype.guest g
JOIN vibetype.contact c ON c.id = g.contact_id,
_blocked
WHERE NOT (c.created_by = ANY(_blocked.ids))
AND (c.account_id IS NULL OR NOT (c.account_id = ANY(_blocked.ids)));
```

The `ANY(array)` ScalarArrayOp on a cross-joined single-row CTE is faster than correlated `NOT EXISTS` subqueries against a multi-row CTE because the array scan has lower per-row overhead than probing a materialized CTE.

**NULL safety**: `NOT (NULL = ANY(array))` evaluates to NULL (treated as FALSE by policies), which incorrectly excludes rows with NULL columns. Add an explicit `IS NULL OR` guard for nullable columns like `contact.account_id`.

### When SECURITY DEFINER helpers are still needed

Some policy checks require querying other RLS-protected tables (for example, the guest policy needs to check the `contact` and `event` tables).
In these cases, the check must run with elevated privileges to avoid infinite RLS recursion.

The recommended approach is to extract these cross-table checks into **focused SECURITY DEFINER helper functions** that return arrays of IDs, rather than wrapping the entire policy:

```sql
-- Good: focused helper returns array, inlined in policy
CREATE FUNCTION vibetype_private.events_with_claimed_attendance() RETURNS UUID[]
LANGUAGE sql STABLE STRICT SECURITY DEFINER
AS $$ ... $$;

CREATE POLICY event_select ON vibetype.event FOR SELECT
USING (
... OR EXISTS (
SELECT 1 FROM unnest(vibetype_private.events_with_claimed_attendance()) AS att
WHERE att = event.id
)
);

-- Avoid: wrapper function takes entire row, prevents SubPlan optimization
CREATE FUNCTION vibetype_private.event_policy_select(e vibetype.event) RETURNS boolean
LANGUAGE sql STABLE STRICT SECURITY DEFINER
AS $$ ... $$;

CREATE POLICY event_select ON vibetype.event FOR SELECT
USING (vibetype_private.event_policy_select(event.*));
```
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
"packageManager": "pnpm@10.33.0",
"private": true,
"scripts": {
"benchmark": "./test/benchmark/benchmark.sh",
"benchmark:compare": "./test/benchmark/benchmark.sh --compare",
"deploy": "pnpm run sqitch deploy",
"revert": "pnpm run sqitch revert",
"sqitch": "./src/sqitch",
Expand All @@ -18,5 +20,5 @@
"test:update": "pnpm run test --update"
},
"type": "module",
"version": "10.0.8"
"version": "11.0.0-beta.4"
}
23 changes: 13 additions & 10 deletions src/deploy/function_account_block_ids.sql
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
BEGIN;

CREATE FUNCTION vibetype_private.account_block_ids() RETURNS TABLE(id uuid)
CREATE FUNCTION vibetype_private.account_block_ids() RETURNS UUID[]
LANGUAGE sql STABLE STRICT SECURITY DEFINER
AS $$
-- users blocked by the current user
SELECT blocked_account_id
FROM vibetype.account_block
WHERE created_by = vibetype.invoker_account_id()
UNION ALL
-- users who blocked the current user
SELECT created_by
FROM vibetype.account_block
WHERE blocked_account_id = vibetype.invoker_account_id();
SELECT COALESCE(array_agg(blocked_id), ARRAY[]::UUID[])
FROM (
-- users blocked by the current user
SELECT blocked_account_id AS blocked_id
FROM vibetype.account_block
WHERE created_by = vibetype.invoker_account_id()
UNION ALL
-- users who blocked the current user
SELECT created_by AS blocked_id
FROM vibetype.account_block
WHERE blocked_account_id = vibetype.invoker_account_id()
) sub;
$$;

COMMENT ON FUNCTION vibetype_private.account_block_ids() IS 'Returns all account ids being blocked by the invoker and all accounts that blocked the invoker.';
Expand Down
Loading
Loading