Skip to content

Constraint dedup compares callables by id() with no comment on why it is sound #129

Open

Description

@cdwilson

Constraint deduplication compares callables by id() with no comment explaining why that is sound.

What happens

_unique_constraints() deduplicates on (frozenset(fields), id(constraint.key)) — src/tinydantic/_model.py:1282-1285:

identity = (
    frozenset(constraint.fields),
    None if constraint.key is None else id(constraint.key),
)

Identity, not equality. Two structurally identical key=lambda v: v.casefold() constraints are kept as distinct constraints and both enforce — harmless, just extra scans. The merge is also recomputed on every write rather than cached.

Why it matters

The method's docstring does document the collapse semantics ("same field set and same key callable … collapse to one; the same field set with different keys yields distinct constraints that all enforce"). What it does not explain is why id()-based identity is sound here — the callables are held alive by the class config, so the ids cannot be recycled underneath the set. That is the load-bearing fact, and a reader who knows the usual id() reuse hazard will stop at this line and wonder.

id() on a possibly-dead object is a classic bug shape. The code is correct; it just does not say so.

What to do

  1. Add the comment explaining the lifetime argument.
  2. Consider caching the merged constraint list on the class instead of recomputing it per write — this is on the hot path that the uniqueness scan already runs.

Low priority: no user-visible defect, only a comprehension and a small efficiency gap.

Reproduction

from tinydb import TinyDB
from tinydb.storages import MemoryStorage

from tinydantic import TinydanticModel, UniqueConstraint

db = TinyDB(storage=MemoryStorage)
same = str.casefold


class SameObject(
    TinydanticModel,
    database=db,
    constraints=(UniqueConstraint("v", key=same), UniqueConstraint("v", key=same)),
):
    v: str


class SameSource(
    TinydanticModel,
    database=db,
    constraints=(
        UniqueConstraint("v", key=lambda v: v.casefold()),
        UniqueConstraint("v", key=lambda v: v.casefold()),
    ),
):
    v: str


for model in (SameObject, SameSource):
    print(f"{model.__name__:11} {len(model._unique_constraints())} constraint(s)")

Run with python repro.py.

Actual output:

SameObject  1 constraint(s)
SameSource  2 constraint(s)

Two constraints that do the same thing survive as two because id() separates them. Both enforce, so the result is correct — just scanned twice.

Expected: no behavior change is being asked for. The id() identity is sound (the callables are held alive by the class config, so ids cannot be recycled underneath the set) and the finding is that the code never says so.

Not a storage cost: ProfilingMiddleware (added in #141) shows both models at 3 storage reads per insert(). The duplicate costs an extra in-memory pass over the table already read, not another read — so the efficiency half of this issue is smaller than "extra scans" suggests.

Environment: tinydantic 0.4.0 at 049f261, Python 3.10.20, TinyDB 4.9.0, pydantic 2.13.4, macOS. Run 2026-08-13.


Verified at e7d0ed0 on 2026-08-12. Originally finding L6 of the 2026-08-06 internal review.

Assisted-by: Claude:claude-opus-5

Activity

  1. added
    polishSmall cleanup, no behavior change
    ai-assistedCreated or investigated with help from an AI agent
    on Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ai-assistedCreated or investigated with help from an AI agentpolishSmall cleanup, no behavior change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions