New rule: do something about the workarounds to "no subqueries in check constraints"

Open
#1,228 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Feature
Clarity
Needs clarification
Activity status
Quiet
Tech stack
postgresql, rust, sql
Domain
databases, tooling

Research direction

No repository files or tests are named in the issue. Start by reviewing the existing check-constraint linting discussed in #1226 and validating the proposed PostgreSQL catalog and dependency query. Done means agreeing on a precise rule for detecting unsafe workarounds and defining implementation and regression coverage for it.

Written by the indexing model from the issue text.

Description

enhancement

Extremely amusingly, I found #1226 which is suggesting linting against the case postgres catches of doing silly business in check constraints. This issue is about stopping the workarounds to that runtime error, because it turns out those are also a problem: they fail in pg_restore.

I'm raising this issue primarily to highlight that it's a way you can mess up (I blame AI for this landing in our prod environment though!), rather than a remotely important feature request, though I think it is actionable.

Quoth the documentation: https://www.postgresql.org/docs/current/ddl-constraints.html

PostgreSQL does not support CHECK constraints that reference table data other than the new or updated row being checked. While a CHECK constraint that violates this rule may appear to work in simple tests, it cannot guarantee that the database will not reach a state in which the constraint condition is false (due to subsequent changes of the other row(s) involved). This would cause a database dump and restore to fail. The restore could fail even when the complete database state is consistent with the constraint, due to rows not being loaded in an order that will satisfy the constraint. If possible, use UNIQUE, EXCLUDE, or FOREIGN KEY constraints to express cross-row and cross-table restrictions.

This syntactic restriction by Postgres can be dodged by using a function:

CREATE TABLE invoices (id uuid PRIMARY KEY, organization_id uuid);
CREATE TABLE payout_accounts (id uuid PRIMARY KEY, organization_id uuid);

CREATE FUNCTION org_matches(p_invoice_id uuid, p_destination_account_id uuid)
RETURNS boolean LANGUAGE sql STABLE AS $$
  SELECT p_invoice_id IS NULL
    OR EXISTS (SELECT 1 FROM invoices inv
               JOIN payout_accounts pa ON pa.id = p_destination_account_id
               WHERE inv.id = p_invoice_id AND inv.organization_id = pa.organization_id);
$$;

CREATE TABLE invoice_line_items (
  id uuid PRIMARY KEY,
  invoice_id uuid,
  destination_account_id uuid,
  CONSTRAINT org_check CHECK (org_matches(invoice_id, destination_account_id))
);

INSERT INTO invoices          VALUES ('11111111-1111-1111-1111-111111111111','99999999-9999-9999-9999-999999999999');
INSERT INTO payout_accounts   VALUES ('22222222-2222-2222-2222-222222222222','99999999-9999-9999-9999-999999999999');
INSERT INTO invoice_line_items VALUES ('33333333-3333-3333-3333-333333333333','11111111-1111-1111-1111-111111111111','22222222-2222-2222-2222-222222222222');

Unfortunately, to block this with perfect precision and recall, the linter needs to see all functions that are defined, and probably still is busted with dynamic accesses of some kind.

However, one thing that could work is to ban non-builtin functions in check constraints. Thoughts about this idea? It would cause false positives for sure, but it might be okay.

An alternative is runtime analysis in the migrator (by refining the carelessly vibecoded query below), but based on my research on a very large codebase, the simpler "forbid all non-builtin functions" path should have <10 false positives on thousands of migrations for us at least.

extremely vibecoded query to soundly find them all, maybe?? not tested! 
WITH RECURSIVE "check_constraints" /* Step 1: All CHECK constraints with their home table */ AS (
  SELECT
    "con"."oid" AS "constraint_oid",
    "con"."conname" AS "constraint_name",
    "con"."conrelid" AS "home_table_oid",
    "c"."relname" AS "home_table_name",
    "n"."nspname" AS "home_schema_name",
    PG_GET_CONSTRAINTDEF("con"."oid") AS "constraint_definition"
  FROM "pg_constraint" AS "con"
  JOIN "pg_class" AS "c"
    ON "c"."oid" = "con"."conrelid"
  JOIN "pg_namespace" AS "n"
    ON "n"."oid" = "c"."relnamespace"
  WHERE
    "con"."contype" = 'c'
    AND NOT "n"."nspname" IN ('pg_catalog', 'information_schema')
), "constraint_functions" /* Step 2: Functions directly referenced by those CHECK constraints (non-builtin only) */ AS (
  SELECT
    "cc"."constraint_oid",
    "p"."oid" AS "func_oid",
    "p"."proname" AS "func_name",
    "fn"."nspname" AS "func_schema"
  FROM "check_constraints" AS "cc"
  JOIN "pg_depend" AS "dep"
    ON "dep"."objid" = "cc"."constraint_oid"
    AND "dep"."classid" = CAST('pg_constraint' AS REGCLASS)
    AND "dep"."refclassid" = CAST('pg_proc' AS REGCLASS)
  JOIN "pg_proc" AS "p"
    ON "p"."oid" = "dep"."refobjid"
  JOIN "pg_namespace" AS "fn"
    ON "fn"."oid" = "p"."pronamespace"
  WHERE
    NOT "fn"."nspname" IN ('pg_catalog', 'pg_internal')
), "all_funcs" /* Step 3: Recursively expand function -> function dependencies */ /*         so we catch functions that call other custom functions */ AS (
  /* Base: functions directly referenced by constraints */
  SELECT
    "cf"."constraint_oid",
    "cf"."func_oid",
    "cf"."func_name",
    "cf"."func_schema"
  FROM "constraint_functions" AS "cf"
  UNION
  /* Recursive: functions called by those functions */
  SELECT
    "af"."constraint_oid",
    "p"."oid",
    "p"."proname",
    "fn"."nspname"
  FROM "all_funcs" AS "af"
  JOIN "pg_depend" AS "dep"
    ON "dep"."objid" = "af"."func_oid"
    AND "dep"."classid" = CAST('pg_proc' AS REGCLASS)
    AND "dep"."refclassid" = CAST('pg_proc' AS REGCLASS)
  JOIN "pg_proc" AS "p"
    ON "p"."oid" = "dep"."refobjid"
  JOIN "pg_namespace" AS "fn"
    ON "fn"."oid" = "p"."pronamespace"
  WHERE
    NOT "fn"."nspname" IN ('pg_catalog', 'pg_internal')
), "func_table_deps" /* Step 4: Tables that each (transitively-reachable) function depends on */ AS (
  SELECT
    "af"."constraint_oid",
    "af"."func_oid",
    "af"."func_name",
    "af"."func_schema",
    "rel"."oid" AS "dep_table_oid",
    "rel"."relname" AS "dep_table_name",
    "tn"."nspname" AS "dep_table_schema"
  FROM "all_funcs" AS "af"
  JOIN "pg_depend" AS "dep"
    ON "dep"."objid" = "af"."func_oid"
    AND "dep"."classid" = CAST('pg_proc' AS REGCLASS)
    AND "dep"."refclassid" = CAST('pg_class' AS REGCLASS)
  JOIN "pg_class" AS "rel"
    ON "rel"."oid" = "dep"."refobjid"
    AND "rel"."relkind" = 'r' /* ordinary tables only */
  JOIN "pg_namespace" AS "tn"
    ON "tn"."oid" = "rel"."relnamespace"
  WHERE
    NOT "tn"."nspname" IN ('pg_catalog', 'information_schema', 'pg_internal')
)
/* Step 5: Flag constraints where a referenced table != the constraint's home table */
SELECT
  "cc"."home_schema_name" AS "schema",
  "cc"."home_table_name" AS "table",
  "cc"."constraint_name",
  "ftd"."func_schema" || '.' || "ftd"."func_name" AS "offending_function",
  "ftd"."dep_table_schema" || '.' || "ftd"."dep_table_name" AS "cross_table_reference",
  "cc"."constraint_definition"
FROM "check_constraints" AS "cc"
JOIN "func_table_deps" AS "ftd"
  ON "ftd"."constraint_oid" = "cc"."constraint_oid"
WHERE
  "ftd"."dep_table_oid" <> "cc"."home_table_oid" /* the cross-table condition */
ORDER BY
  "cc"."home_schema_name",
  "cc"."home_table_name",
  "cc"."constraint_name"
Dominant language
Rust
Stars
1.2k
Forks
70
Avg merge
52m
Merged PRs (30d)
47

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from sbdchd/squawk

All issues in sbdchd/squawk

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.