Skip to content

fix(postgres): stop a naming convention from qualifying the check constraints twice - #37

Merged
AlexeyShalaev merged 2 commits into
masterfrom
fix/constraint-names
Sep 14, 2026
Merged

AlexeyShalaev merged 2 commits into
masterfrom
fix/constraint-names

Conversation

@AlexeyShalaev

Copy link
Copy Markdown
Member

Closes #36.

Problem

get_event_constraints declared every check constraint with a finished ck_<table>_<rule> name. A MetaData whose ck naming convention interpolates %(constraint_name)s — sqlalchemy-foundation-kit's "%(table_name)s_%(constraint_name)s_check", or the "ck_%(table_name)s_%(constraint_name)s" SQLAlchemy's documentation recommends — qualifies that name a second time. Six of the twelve then run past PostgreSQL's 63 bytes and SQLAlchemy truncates them with a hash; the shortest offender is on the plain outbox, not the partitioned one:

outbox_events_ck_outbox_events_completed_status_consistency_check   65 bytes
→ outbox_events_ck_outbox_events_completed_status_consist_a784
outbox_events_partitioned_ck_outbox_events_p_completed_status_consistency_check   79 bytes
→ outbox_events_partitioned_ck_outbox_events_p_completed__04a8

A name like that matches neither the prefix nor the suffix alembic-gauntlet derives from the convention, so test_naming_conventions fails on a table the consumer did not write.

Design

The helper takes the MetaData and does what SQLAlchemy is about to do with the name — the branch in sqlalchemy/sql/naming.py is "rewrite an explicit name only when the rule interpolates constraint_name". When it does, get_event_constraints hands over the bare rule (attempts_valid, completed_status_consistency, lock_consistency) and the convention builds the name; when it does not, the name is ck_<table>_<rule> exactly as before.

MetaData longest name bytes
none / default ck_outbox_events_p_completed_status_consistency (unchanged) 47
foundation-kit outbox_events_partitioned_completed_status_consistency_check 60
SQLAlchemy docs ck_outbox_events_partitioned_completed_status_consistency 57

Under SQLAlchemy's convention the non-partitioned tables come out byte for byte as today.

The four bases build __table_args__ in a declared_attr.directive that passes cls.metadata, and get_event_constraints gains *, metadata=None for anyone composing __table_args__ by hand. The declared_attr fixes a second thing the reproduction ran into first: the tuple was built once at import, its Index objects were attached to the first table that used them, and binding the bases to a second DeclarativeBase in the same process raised ArgumentError: Index 'idx_outbox_events_pending_fetch' is against table 'outbox_events'. Each concrete model now gets its own.

Indexes are left as they are on purpose. Neither convention's ix rule interpolates constraint_name, so SQLAlchemy keeps an explicitly named Index, and an index name is schema-scoped in PostgreSQL — a finished idx_<table>_… is the right name for one. A test pins that they do not move under either convention.

Rejected

sqlalchemy.schema.conv() around the current names stops the doubling but leaves ck_… in the catalog; gauntlet derives _check as the allowed suffix from the foundation-kit rule and keeps chk_ as the allowed prefix, so the same test fails for the same reporter. Bare rules unconditionally would create a constraint literally called attempts_valid for everyone with a plain DeclarativeBase and make every database built from the documented DDL differ from a fresh one for no reason. Documenting an exclusion fixes nothing.

Compatibility

Nobody without a qualifying ck rule sees a different name; direct callers of the helper are unchanged unless they pass metadata. The one shape change: OutboxEventDBBase.__table_args__ and its three siblings are declared_attrs now, so reading the tuple off the abstract class no longer works — that was never documented; the documented way is the helper. A database that already carries truncated names keeps them until renamed; alembic's autogenerate does not compare CHECK constraints unless alembic.ext.checkconstraint_byname (1.19+) is enabled, so docs/migrations.md now carries the query that finds them and the RENAME CONSTRAINT.

Verification

uv run ruff check . && uv run ruff format --check . && uv run mypy omni_box
  All checks passed! / 175 files already formatted / Success: no issues found in 79 source files
uv run pytest -m unit --cov=omni_box --cov-fail-under=90
  768 passed, 3 skipped, 95 deselected — Total coverage: 98.62%
uv run pytest tests/integration/postgres -m integration        (Docker, postgres:17-alpine)
  60 passed

The new integration test binds the bases to a MetaData(naming_convention=<foundation-kit>, schema=…), runs create_all against PostgreSQL 17 and reads the names back through inspect(conn).get_check_constraints — the same call gauntlet uses.

With origin/master's orm.py swapped in, the new tests fail: every test that binds the bases to a second DeclarativeBase with ArgumentError: Index 'idx_outbox_events_pending_fetch' is against table 'outbox_events' (the one that runs first in the process passes — which is why the existing suite never hit it), the helper-level ones with TypeError: unexpected keyword argument 'metadata'. The truncation itself, on the old helper attached to a bare Table under the foundation-kit convention:

outbox_events              outbox_events_ck_outbox_events_completed_status_consistency_check                 65  ddl=outbox_events_ck_outbox_events_completed_status_consist_a784
outbox_events_partitioned  outbox_events_partitioned_ck_outbox_events_p_completed_status_consistency_check   79  ddl=outbox_events_partitioned_ck_outbox_events_p_completed__04a8

…straints twice

get_event_constraints declared every check constraint with a finished
ck_<table>_<rule> name. A MetaData whose ck naming convention interpolates
%(constraint_name)s — sqlalchemy-foundation-kit's
"%(table_name)s_%(constraint_name)s_check", or the convention SQLAlchemy's
documentation recommends — qualifies that name a second time, six of the
twelve run past PostgreSQL's 63 bytes, and SQLAlchemy truncates them with a
hash: outbox_events_ck_outbox_events_completed_status_consist_a784. The
name in the catalog then no longer matches the convention that produced it,
which is what alembic-gauntlet's test_naming_conventions checks.

The helper now takes the MetaData and does what SQLAlchemy is about to do
with the name: when the ck rule interpolates the constraint name it hands
over the bare rule (attempts_valid, completed_status_consistency,
lock_consistency) and the convention builds the name; otherwise it declares
ck_<table>_<rule> as before. The four bases build __table_args__ in a
declared_attr that passes cls.metadata, which also gives each concrete
model its own Index objects — the shared tuple could only ever be bound to
one DeclarativeBase per process.

Closes #36
@AlexeyShalaev

Copy link
Copy Markdown
Member Author

Reporter here — verified against the shape that produced the report, and it holds.

Our models inherit from two parents, the library base and our own declarative base, and the
MetaData carrying the convention comes from the latter:

class InboxEventDB(BaseTable, InboxEventPartitionedDBBase): ...

That was the thing I could not tell from the diff: whether @declared_attr.directive on the
abstract base would see a convention that arrives through the other parent. It does. Built against
fix/constraint-names with no changes on our side and no __table_args__ override:

inbox_events_partitioned_attempts_valid_check                 45
inbox_events_partitioned_completed_status_consistency_check   59
inbox_events_partitioned_lock_consistency_check               47
outbox_events_partitioned_attempts_valid_check                46
outbox_events_partitioned_completed_status_consistency_check  60
outbox_events_partitioned_lock_consistency_check              48

Six of six inside 63 bytes, all ending in _check, all in the convention's own shape. The local
workaround we were carrying — wrapping the declared names in sqlalchemy.schema.conv and appending
the suffix by hand, across fifteen ORM modules — is deleted by this, and the names it produced were
in omni-box's shape rather than the schema's. Yours is the better result.

The ALTER TABLE ... RENAME CONSTRAINT note is the right call for existing databases; ours are not
deployed yet, so we only regenerate the initial migrations.

One coordination note for whoever picks the release: __table_args__ becoming a declared_attr
means a consumer that reads it as a plain class attribute (as our workaround did) breaks at import
on upgrade. Loud and immediate, which is the right failure, but worth a line in the release notes
next to the rename SQL.

@AlexeyShalaev

Copy link
Copy Markdown
Member Author

Good to know it holds through a second parent — that was the case I could only reason about, not run.

The changelog here is generated from the commit subject, so the release notes cannot carry the line; the migration guide is where an upgrade is described, and 46c8193 puts it right under the RENAME CONSTRAINT: reading __table_args__ off the abstract base now raises AttributeError: ... has no attribute 'metadata' at import, delete the workaround. I ran your exact access against the branch to get the message verbatim rather than guess it.

@AlexeyShalaev
AlexeyShalaev merged commit e125d39 into master Sep 14, 2026
5 checks passed
@AlexeyShalaev
AlexeyShalaev deleted the fix/constraint-names branch September 14, 2026 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Event check-constraint names already carry the table, so any naming_convention truncates them past 63 bytes

1 participant