bstore-j/CODE_REVIEW.md

bstore-java Code Review

Review date: 2026-03-17

This review focuses on correctness, architectural fit to the current goals, and remaining risks before using bstore-java as the reference implementation for other language ports.

Findings

No new correctness findings were discovered in this final pass after the latest meta-history fixes.

Non-Findings

These areas looked acceptable for the current scope:

• dbo primary-key semantics are now explicit and much less error-prone.
• SQL IN handling is parameterized.
• Action.first() cleanup is fixed.
• SQLReader.hasNext() no longer advances repeatedly.
• dbo.meta is now appropriately lean for the current goals.
• ChangeEvent as canonical history and DataOriginator as module registry is a coherent split.
• terminal.save(changeEvent) now persists the event row without implicitly applying schema changes.
• apply_changes(...) now logs pending events before attempting structural changes, so failed batches do not lose the canonical change record.
• upgradeMetaSchema(...) now respects the caller-supplied originatorId and migrationId.

Current Limits That Seem Acceptable

These are limitations, but they appear aligned with the current requirements:

• field rename is treated as drop + add rather than a first-class rename
• schema moves are rejected explicitly
• PK/auto-increment/unique/index rewrites are not yet supported in apply_changes(...)
• downgrade support is schema-shape oriented, not a full data-preserving rollback engine
• migration batches are still best-effort rather than transactional across all DDL plus history writes; the log is preserved, but partial application can still occur on backends or change sets that are not fully transactional

Suggested TODO List

Priority 1

• Decide whether to add optional transactional migration batches on backends that support transactional DDL, or keep the current best-effort semantics as the long-term contract.
• Add a short public example showing when to use terminal.save(changeEvent) versus meta.apply_changes(...).

Priority 2

• If ChangeEventHero is no longer part of the intended public design, remove it or explicitly deprecate it to avoid confusion.
• Consider exposing an explicit "planned but unapplied" query path for pending changes now that unapplied events are preserved in the log.

Priority 3

• Review old exploratory tests and either remove them permanently or mine any remaining useful cases into the curated suites.
• Prepare a concise cross-language contract document derived from the current Java dbo and meta behavior before porting further.

Bottom Line

The project is in a strong reference-implementation state for the original goals. The remaining important risks are mostly around future migration ergonomics and transaction semantics, not everyday CRUD correctness or core meta design.