Skip to content

Distinguish composite types of user types from built-in composites - #1298

Merged
cberner merged 1 commit into
masterfrom
claude/codebase-bug-review-p9qemy
Jul 9, 2026
Merged

Distinguish composite types of user types from built-in composites#1298
cberner merged 1 commit into
masterfrom
claude/codebase-bug-review-p9qemy

Conversation

@cberner

@cberner cberner commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Problem

A composite type (Option, Vec, tuple, or array) built its TypeName from only the inner type's name string, discarding the inner type's user-defined-vs-built-in classification. So Option<u32> (built-in) and Option<MyType>, where MyType is a user type named "u32", produced byte-identical type names.

A table created with one could then be opened as the other with no error, silently reading the stored bytes under the wrong encoding and yielding wrong results or unreachable keys. The classification byte exists precisely to keep a user type named "u32" from colliding with the built-in u32, but that protection was lost one nesting level down.

Fix

Bubble the classification up: a composite that wraps a user-defined type is now itself classified user-defined, keeping the same name string. Composites of only built-in types are byte-for-byte unchanged, so existing databases open unchanged.

To keep tables created by older versions readable, TypeName carries an in-memory-only record of the classification it was bubbled from (never serialized), and check_match accepts that prior spelling in addition to the current one:

  • For fixed-width composites the prior spelling is Internal, which also matches redb 2.6.
  • For variable-width tuples it is Internal2 only, so redb 2.6's incompatible variable-tuple encoding is still correctly rejected rather than silently misread.
  • A flat user-defined type records no such classification, so this never lets a user type named "u32" alias the built-in u32 at the top level.

The Value/Key public traits are untouched — the whole mechanism lives in TypeName plus the composite type_name() impls (which already had to change for the fix).

Known limitation

An old Option<user-type> table is byte-identical on disk to a built-in Option<u32> table, so a name-colliding user type can still open a built-in table in the reverse direction. This is inherent to preserving backwards compatibility and only reachable if a user names their type to collide with a built-in (which the docs already advise against). It is strictly better than before, where the alias happened unconditionally in both directions.

Tests

  • Crate-internal unit tests in src/types.rs: built-in composites keep their exact classification (unchanged on-disk bytes); a composite of a name-colliding user type no longer equals its built-in counterpart; matches_legacy accepts the right Internal/Internal2 prior spellings; and a flat user type never legacy-matches an Internal name.
  • Cross-version tests in tests/backward_compatibility.rs using the real redb 2.6.0 dev-dependency: an Option<user-type> table written by redb 2.6 still opens and reads back correctly, and an Option<user-type> table can no longer be opened as the built-in Option<u32> (returns TableError::TableTypeMismatch).

cargo fmt --check, cargo clippy --all-targets --all-features, and the full cargo test --all-features suite all pass.

🤖 Generated with Claude Code


Generated by Claude Code

@cberner
cberner force-pushed the claude/codebase-bug-review-p9qemy branch from 438edb2 to b3a1d3c Compare July 9, 2026 21:15
A composite type (`Option`, `Vec`, tuple, or array) built its `TypeName`
from only the inner type's name string, discarding the inner type's
user-defined-vs-built-in classification. So `Option<u32>` (built-in) and
`Option<MyType>`, where `MyType` is a user type named "u32", produced
byte-identical type names. A table created with one could then be opened
as the other with no error, silently reading the stored bytes under the
wrong encoding and yielding wrong results or unreachable keys. The
classification byte exists precisely to keep a user type named "u32"
from colliding with the built-in `u32`, but that protection was lost one
nesting level down.

Bubble the classification up: a composite that wraps a user-defined type
is now itself classified user-defined, keeping the same name string.
Composites of only built-in types are byte-for-byte unchanged, so
existing databases open unchanged.

Tables created by older versions stored such a composite with the
non-bubbled classification. To keep them readable, TypeName carries an
in-memory-only record of the classification it was bubbled from (never
serialized), and check_match accepts that prior spelling in addition to
the current one. For fixed-width composites the prior spelling is
`Internal`, which also matches redb 2.6; for variable-width tuples it is
`Internal2` only, so redb 2.6's incompatible variable-tuple encoding is
still correctly rejected rather than silently misread. A flat
user-defined type records no such classification, so this never lets a
user type named "u32" alias the built-in `u32` at the top level.

Assisted-by: Claude Code
https://claude.ai/code/session_01G4cggr4Aoc9BVkEndj5Uyf
@cberner
cberner force-pushed the claude/codebase-bug-review-p9qemy branch from b3a1d3c to 1eddc21 Compare July 9, 2026 21:17
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.51613% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.22%. Comparing base (b6b56aa) to head (1eddc21).

Files with missing lines Patch % Lines
src/types.rs 87.96% 13 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #1298    +/-   ##
========================================
  Coverage   90.21%   90.22%            
========================================
  Files          36       36            
  Lines       16330    16447   +117     
========================================
+ Hits        14732    14839   +107     
- Misses       1598     1608    +10     
Files with missing lines Coverage Δ
src/complex_types.rs 87.50% <100.00%> (+0.40%) ⬆️
src/tree_store/table_tree_base.rs 89.38% <100.00%> (+0.21%) ⬆️
src/tuple_types.rs 91.74% <100.00%> (+2.95%) ⬆️
src/types.rs 96.22% <87.96%> (-2.84%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cberner
cberner merged commit 4388342 into master Jul 9, 2026
8 checks passed
@cberner
cberner deleted the claude/codebase-bug-review-p9qemy branch July 9, 2026 22:06
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.

1 participant