fix: translate errors from the sqlite3 error type instead of JSON - #247
Open
davidpavlovschi wants to merge 1 commit into
Open
fix: translate errors from the sqlite3 error type instead of JSON#247davidpavlovschi wants to merge 1 commit into
davidpavlovschi wants to merge 1 commit into
Conversation
Translate round-tripped the error through encoding/json and read the extended result code off the resulting object. That has two problems. It matches on field names rather than on type, so any error carrying Code/ExtendedCode/SystemErrno fields is translated even when it comes from an unrelated package, and a real sqlite error stops being recognised as soon as a callback or plugin wraps it. Read the extended result code from sqlite3.Error through errors.As so the whole error chain is searched and only sqlite errors match. sqlite3.Error is declared in a file that imports C, so Translate is split by build tag. Without cgo the driver cannot open a connection at all, so the non-cgo build returns the error untouched.
davidpavlovschi
added a commit
to glebarez/sqlite
that referenced
this pull request
Aug 2, 2026
…nslation (#155) * Port 11 behaviour fixes from go-gorm/sqlite (parity to upstream 525c431) Ports every behaviour-changing upstream commit since 7544227, verified against the modernc backend: - 13be5e3 (go-gorm#236): preserve indexes, triggers, table options and views across table rebuilds - d450667 (go-gorm#235): HasColumn matches exactly via pragma_table_info - 69c4707 (go-gorm#230): generated (computed) columns via the generated tag - fc0cfd3 (go-gorm#207): parse CHECK/CONSTRAINT/FOREIGN KEY table-level lines - 5406e41 (go-gorm#198): case/whitespace-tolerant constraint matching - 02b8e06 (go-gorm#193): parseAllColumns state machine for composite keys - 7230345 (go-gorm#229): disable foreign keys during DropColumn - 6f07b51: don't overwrite caller-registered clause builders - 75dbf08 (go-gorm#222): accept tab between column name and type - bbca3b3 (go-gorm#185): exported Config and New(Config) constructor - 5df1f76 (go-gorm#219): remove stray Debug() from GetIndexes migrator.go, ddlmod.go and ddlmod_parse_all_columns.go are taken from upstream verbatim; sqlite.go is hand-merged to keep this fork's imports, DriverName, and its error-code-based Translate. Upstream's test files for these paths are ported unchanged. Refs #152 * Bump gorm and the pure-Go sqlite backend gorm.io/gorm 1.25.7 to 1.31.2, github.com/glebarez/go-sqlite 1.21.2 to 1.22.0, which pulls modernc.org/sqlite 1.23.1 to 1.28.0. go mod tidy refreshes the indirect set. The go directive stays at 1.18: gorm 1.31.2 declares go 1.18 and go-sqlite 1.22.0 declares go 1.17, so nothing forces a raise. Test counts are unchanged, 15 top-level tests and 70 including subtests, all passing. Refs #152 * Document upstream parity in the README States the last upstream commit merged (525c431), the three divergences that are deliberate (pure-Go backend imports, DriverName "sqlite", error-code-based Translate), and the rule that backend-agnostic fixes go to go-gorm/sqlite first and come back as ports. Refs #152 * Translate CHECK constraint failures to gorm.ErrCheckConstraintViolated SQLITE_CONSTRAINT_CHECK (extended code 275) fell through Translate and reached the caller as a raw driver error, so TranslateError users had to string-match "CHECK constraint failed" to detect it. UNIQUE, PRIMARY KEY and FOREIGN KEY were already mapped. The test drives a real in-memory database: it migrates a model carrying a CHECK constraint, asserts the constraint is in the table DDL so a wrong failure cannot pass for the right one, then inserts a row that violates it. Without the mapping the test fails with the raw error, code 275. This mirrors the backport of the go-gorm#247 follow-up. Refs #152 * Fix unquoted constraints during table rebuilds
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
1 participant
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.
I maintain glebarez/sqlite, the pure-Go fork of
this driver. I have been running a parity audit between the two, and I committed in
glebarez#152 to sending anything backend-agnostic here first rather than letting
the fork drift. This is the first of those.
Translatecurrently round-trips the error throughencoding/jsonand reads theextended result code off the decoded object. That has two problems.
It matches on field names rather than on type. Any error carrying
Code,ExtendedCodeandSystemErrnofields gets translated even when it comes from acompletely unrelated package. I added a test type whose message is "unrelated error
from another library" and today
Translateturns it intogorm.ErrDuplicatedKey.It also stops recognising a real sqlite error the moment something wraps it.
json.Marshalon a wrapped error produces
{}, so the extended code reads back as0. Any callback,hook or plugin that adds context to the driver error before it reaches
AddErrorlosestranslation silently.
This reads the extended result code from
sqlite3.Errorthrougherrors.As, so thewhole error chain is searched and only real sqlite errors match. Both
sqlite3.Errorand
*sqlite3.Errorare probed, so the pointer case that worked before keeps working.About the comment saying the go-sqlite3 error type is avoided because it needs cgo: I
checked, and that reason is correct.
sqlite3.Erroris declared in a file that importsC, so it does not exist under
CGO_ENABLED=0, and a plain type switch would break thatbuild. So
Translateis split by build tag. The non-cgo build returns the erroruntouched, which is the right answer there: without cgo the driver cannot open a
connection at all, so no sqlite error ever reaches the function. That also removes the
false positives on that build, where the JSON heuristic could only ever misfire.
ErrMessageis exported, so I kept it and marked it deprecated rather than deleting it.One compatibility note. If someone points this Dialector at a pure-Go sqlite driver via
DriverNameand builds withCGO_ENABLED=0, translation becomes a no-op. That is not aregression:
modernc.org/sqlite's error type has unexported fields, sojson.Marshalalready produced
{}and the old code did not translate it either.There was no test for
Translatebefore this, so all seven tests are new. Two of themfail against the old implementation and pass against the new one. The other five pass
against both, which is what shows existing behaviour is unchanged.
go test ./...goes from 70 passing to 77, no failures.gofmt,go vet,CGO_ENABLED=0 go build ./...andCGO_ENABLED=0 go vet ./...are all clean.