feat: embed dal.DB so Database satisfies the sealed interface - #4
Merged
Conversation
dal.DB is now sealed by an unexported marker method (dal-go/dalgo v0.64.2), produced only by dal.NewDB. Database used to name its dalgo2sql delegate as a plain field (innerDB dal.DB), which cannot promote that marker method, so *Database stopped satisfying dal.DB. Embedding dal.DB instead promotes it along with the rest of the interface, matching the decorator pattern dal.DB's own doc comment describes. database_dal.go's forwarding methods move from d.innerDB.X to d.DB.X. Set/SetMulti/Insert/Upsert/Delete/DeleteMulti/Update/UpdateMulti need more than a rename: dalgo2sql's backend does not implement dal.WriteSession in full at the database level (no database-level InsertMulti or UpdateRecord), so dal.NewDB never wraps it in the validating write pipeline there, and a plain assertion against d.DB's dynamic type finds none of these methods at all. dal.As recovers dalgo2sql's concrete backend, which does implement them directly, the same way dal.WithoutValidation recovers an unvalidated write session. These direct, non-transactional writes were not run through validation before this change either, since dalgo2sql's database-level write path has no BeforeSave call of its own; writes made through RunReadwriteTransaction are validated, because that goes through d.DB's own RunReadwriteTransaction. Embedding dal.DB alongside dal.ConcurrencyAvailable makes SupportsConcurrentConnections ambiguous (both declare it at the same promotion depth), so Database now defines it explicitly. Bumps github.com/dal-go/dalgo to v0.64.2 and github.com/dal-go/dalgo2sql to v0.10.0, the already-converted release these adapters build on. Adds conformance_test.go wiring dalgotest.RunConformance behind this package's existing DALGO2POSTGRES_TEST_DSN env-gate, matching every other DB-backed test here. No live PostgreSQL server was available to run it in this change; it skips exactly like the rest of the suite until DALGO2POSTGRES_TEST_DSN is set, including in this repo's CI, which has no PostgreSQL service container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkkrXdtf8mU2GRo2hHsHT1 Signed-off-by: Alexander Trakhimenok <alex@trakhimenok.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
dal.DB(dal-go/dalgo v0.64.2) is now sealed by an unexported marker method, produced only bydal.NewDB.Databaseused to name its dalgo2sql delegate as a plain field (innerDB dal.DB), which cannot promote that marker method, so*Databasestopped satisfyingdal.DB. Fixed by embeddingdal.DBinstead of naming it.database_dal.go's pure forwarding methods move fromd.innerDB.Xtod.DB.X.Set/SetMulti/Insert/Upsert/Delete/DeleteMulti/Update/UpdateMulti) need more than a rename: dalgo2sql's backend does not implementdal.WriteSessionin full at the database level (no database-levelInsertMultiorUpdateRecord), sodal.NewDBnever wraps it in the validating write pipeline there —d.DB's dynamic type has none of these methods at all, verified empirically.dal.Asrecovers dalgo2sql's concrete backend, which does implement them directly. This is unchanged behaviour: these direct, non-transactional writes were never validated even before this change, since dalgo2sql's database-level write path has noBeforeSavecall of its own. Writes made throughRunReadwriteTransactionare validated, because that goes throughd.DB's ownRunReadwriteTransaction.dal.DBalongsidedal.ConcurrencyAvailablemakesSupportsConcurrentConnectionsan ambiguous selector (both declare it at the same promotion depth), soDatabasenow defines it explicitly.github.com/dal-go/dalgoto v0.64.2 andgithub.com/dal-go/dalgo2sqlto v0.10.0 (the already-converted release these adapters build on).conformance_test.gowiringdalgotest.RunConformance, gated behind this package's existingDALGO2POSTGRES_TEST_DSNenv var, matching every other DB-backed test here.Known gap (not fixed here, reporting per audit)
There is no
dal.BackendOf-equivalent for a transaction handed into aRunReadwriteTransactionworker, so adapter-specific transaction methods are unreachable from inside a worker. This package doesn't define its own transaction type (it delegates entirely to dalgo2sql's), so it does not hit this gap itself, but it is a real upstream limitation worth tracking.Test plan
gofmt -l .— cleanGOWORK=off go vet ./...— cleanGOWORK=off go test -race -count=1 ./...—ok, but 15 of 16 tests skip (noDALGO2POSTGRES_TEST_DSNin this environment); onlyTestNewDatabase_RejectsBadDSNactually runs and passes (no live server needed). No live PostgreSQL server was available to me, soTestConformancecould not be exercised for real — it also skips.TestConformance) against a live PostgreSQL server withDALGO2POSTGRES_TEST_DSNset, to actually exercise the conformance suite.🤖 Generated with Claude Code
https://claude.ai/code/session_01SkkrXdtf8mU2GRo2hHsHT1