Skip to content

@pgsql/utils constraint builder + pgsql-deparser@18 produce non-enforcing constraints #346

Description

@jefflub-ashby

Summary

t.nodes.constraint() does not set is_enforced. pgsql-deparser@18.x interprets an absent is_enforced as false and emits NOT ENFORCED.

The result is that building a CHECK or FOREIGN KEY constraint with the project's own AST builder, and rendering it with the project's own deparser, produces a constraint that PostgreSQL will never enforce. No error is raised at any stage — the SQL is valid and executes successfully.

This is a disagreement between two first-party packages rather than a bug in either one alone. The deparser's interpretation is correct for parser output; constructed ASTs don't follow the same convention, and the deparser has no way to tell the two apart.

Affected versions

PackageVersion
@pgsql/utils18.2.8builder omits is_enforced
pgsql-deparser18.0.0 → 18.3.6reads absent as NOT ENFORCED
pgsql-deparser17.18.5✅ unaffected

Present from the first pgsql-deparser@18 release through current latest.

Reproduction

Using only first-party packages, following the pattern in the pgsql-deparser README:

import*astfrom"@pgsql/utils";import{deparseSyncasdeparse}from"pgsql-deparser";conststmt=t.nodes.alterTableStmt({relation: t.nodes.rangeVar({relname: "t",inh: true,relpersistence: "p"}).RangeVar,objtype: "OBJECT_TABLE",cmds: [t.nodes.alterTableCmd({subtype: "AT_AddConstraint",behavior: "DROP_RESTRICT",def: t.nodes.constraint({contype: "CONSTR_CHECK",conname: "x_positive",raw_expr: t.nodes.aExpr({kind: "AEXPR_OP",name: [t.nodes.string({sval: ">"})],lexpr: t.nodes.columnRef({fields: [t.nodes.string({sval: "x"})]}),rexpr: t.nodes.aConst({ival: t.ast.integer({ival: 0})}),}),}),})],});console.log(deparse([{RawStmt: { stmt }}]));

Expected:

ALTERTABLE t ADD CONSTRAINT x_positive CHECK (x >0)

Actual:

ALTERTABLE t ADD CONSTRAINT x_positive CHECK (x >0) NOT ENFORCED

The builder output is:

t.nodes.constraint({contype: "CONSTR_CHECK",conname: "c"})// { Constraint: { contype: 'CONSTR_CHECK', conname: 'c' }} <- no is_enforced

Passing is_enforced: true explicitly is preserved by the builder and produces correct SQL.

Scope

Affects the two constraint types PostgreSQL permits NOT ENFORCED on:

CHECK, builder default ALTER TABLE t ADD CONSTRAINT c CHECK (x > 0) NOT ENFORCED ❌
CHECK, is_enforced: true ALTER TABLE t ADD CONSTRAINT c CHECK (x > 0) ✅
FK, builder default ALTER TABLE t ADD CONSTRAINT f FOREIGN KEY(x) REFERENCES o (id) NOT ENFORCED ❌
FK, is_enforced: true ALTER TABLE t ADD CONSTRAINT f FOREIGN KEY(x) REFERENCES o (id) ✅

CONSTR_UNIQUE and CONSTR_PRIMARY are unaffected. Inline constraints in CREATE TABLE are affected the same way as ALTER TABLE ... ADD CONSTRAINT.

skip_validation is not part of the trigger — is_enforced alone determines it. Where skip_validation: true is set, NOT ENFORCEDreplaces the expected NOT VALID rather than accompanying it:

skip_validationis_enforcedOutput
absentabsentNOT ENFORCED
absenttrue(none)
trueabsentNOT ENFORCED ❌ — expected NOT VALID
truetrueNOT VALID
truefalseNOT ENFORCED

Why the deparser's logic is not itself wrong

libpg_query omits false booleans from its JSON output, so for parsed ASTs an absent is_enforced genuinely does mean not-enforced. Confirmed with pgsql-parser@18.2.6:

Parsed SQLis_enforcedskip_validation
CHECK (x > 0)true
CHECK (x > 0) NOT ENFORCEDundefinedtrue
CHECK (x > 0) ENFORCEDtrue
CHECK (x > 0) NOT VALIDtruetrue

So parse()deparse() round-trips correctly today, including NOT ENFORCED, and the deparser's is_enforced !== true check is right for that path.

Changing the deparser to treat absent as enforced would break this. Verified by applying is_enforced === false to 18.3.6 and re-running round-trips:

✗ in : ALTER TABLE t ADD CONSTRAINT c CHECK (x > 0) NOT ENFORCED
out: ALTER TABLE t ADD CONSTRAINT c CHECK (x > 0) NOT VALID

That silently converts a non-enforced constraint into an enforced one — the same class of failure in the opposite direction. The deparser cannot distinguish "false, omitted by convention" from "not specified by the caller"; both arrive as undefined.

Why this matters

NOT ENFORCED (PG18) means the constraint applies to neither existing nor new rows. Both plausible caller intents are silently violated:

  • A bare CHECK or FK constraint should be fully enforced. It is inert instead.
  • NOT VALID should enforce for new rows while deferring validation of existing ones — the standard pattern for adding a constraint to a large table. It is inert instead, and the intended clause is dropped.

Nothing surfaces the problem: the SQL is valid, applies cleanly, and enforces nothing. For a migration tool this means shipped constraints that never take effect, discoverable only by inspecting the database afterwards.

This is reachable through the documented path — the pgsql-deparser README's primary example constructs an AST with @pgsql/utils and notes it "could have been obtained from any JSON or AST."

Suggested fix

Preferred: default is_enforced: true in the @pgsql/utils constraint builder for CONSTR_CHECK and CONSTR_FOREIGN. This aligns the builder with the serialization convention the deparser already assumes, fixes existing callers with no code change on their side, and leaves the deparser's parser-output handling untouched. It's clear that this is a large, invasive change given how pgsql/utils is built, so it's understandable if you don't want to pursue it.

Alternatives, if that isn't desirable:

  • Add a Typescript type constraint that requires an explicit decision in the builder signature for CONSTR_CHECK and CONSTR_FOREIGN. This would be a breaking change for consumers (though probably fixing a bug in their construction). I can provide a PR for this if you're interested.
  • Add a deparser option (e.g. deparse(ast, { assumeEnforced: true })) for callers constructing ASTs rather than round-tripping.
  • Document in packages/deparser/README.md under ## Options, and as TSDoc on Constraint.is_enforced, that omitting the field on a constructed AST yields NOT ENFORCED.

Documentation alone would leave every consumer needing to know an implicit serialization convention in order to avoid emitting non-enforcing constraints, so a default in the builder seems the more robust resolution.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions