[auto-bump] [no-release-notes] dependency by reltuk#2884
Open
coffeegoddd wants to merge 1 commit into
Open
Conversation
Contributor
|
|
SummaryCoverage spans core database connection and query behavior across normal and adversarial protocol flows, including authentication, simple and prepared statement execution, and bulk data import success and rollback handling. The major gap in health is persistence-on-restart behavior, where schemas using custom data types can fail to come back up after a restart. Not safe to merge yet — this PR appears to introduce a high-severity regression in startup reliability, where valid persisted data can trigger server crashes and database unavailability after restart. Even with broad pass results on online query and protocol paths, a restart-time crash in a real data scenario is a release-blocking risk in code touched by this change. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
![itoqa[bot]](https://avatars.githubusercontent.com/in/2622614?s=60&v=4){kind=small}
| github.com/cockroachdb/apd/v3 v3.2.3 | ||
| github.com/cockroachdb/errors v1.7.5 | ||
| github.com/dolthub/dolt/go v0.40.5-0.20260624213831-feba4a825720 | ||
| github.com/dolthub/dolt/go v0.40.5-0.20260626214637-25552a68b104 |
There was a problem hiding this comment.
Persisted custom type deserialization crashes server on restart
What failed: The process crashes with SIGSEGV during root/table initialization because type deserialization requests a type collection with ctx=nil, and getContextValues dereferences ctx.Session without a nil guard.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: High
- Impact: After creating and persisting ENUM-based schema, restarting the server can crash before it accepts connections, leaving affected databases unavailable until repaired. This blocks normal read/write access for users of those databases.
- Steps to Reproduce:
- Start doltgres with a fresh data directory.
- Create an ENUM type and a table that uses that type, then insert rows.
- Stop the server and restart using the same data directory.
- Observe startup panic before ReadyForQuery with stack through recursiveDeserializeType -> GetTypesCollectionFromContext -> getContextValues.
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: core/context.go assumes ctx is non-nil in getContextValues (line 56), and core/context.go line 421 calls that helper from GetTypesCollectionFromContext without validating ctx. During startup deserialization, server/types/serialization.go line 267 calls GetTypesCollectionFromContext(ctx, "") when ctx may be nil in this dependency path. The PR diff modified go.mod dependency lines (9, 12, 15) and the panic stack shows the new go-mysql-server and dolt versions on this call chain, making this a regression introduced by changed dependency code paths. Smallest practical fix: add explicit nil-context handling in GetTypesCollectionFromContext/getContextValues and/or avoid calling context-dependent type collection lookup from recursiveDeserializeType when ctx is nil during startup deserialization.
- Why this is likely a bug: This is a production-code startup path and the observed panic is a deterministic nil-pointer dereference in repository code, not a harness artifact. A database containing valid persisted custom-type metadata should not make the server crash before it can accept connections.
Relevant code
go.mod:9-12
github.com/dolthub/dolt/go v0.40.5-0.20260626214637-25552a68b104
github.com/dolthub/go-mysql-server v0.20.1-0.20260626203645-7c333f13eacacore/context.go:55-57
func getContextValues(ctx *sql.Context) (*contextValues, error) {
sess := dsess.DSessFromSess(ctx.Session)
if sess.DoltgresSessObj == nil {core/context.go:420-422
func GetTypesCollectionFromContext(ctx *sql.Context, database string) (*typecollection.TypeCollection, error) {
cv, err := getContextValues(ctx)
if err != nil {server/types/serialization.go:266-268
if typeColl == nil {
typeColl, err = GetTypesCollectionFromContext(ctx, "")
if err != nil {Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**High severity — Persisted custom type deserialization crashes server on restart**
**What failed:** The process crashes with SIGSEGV during root/table initialization because type deserialization requests a type collection with ctx=nil, and getContextValues dereferences ctx.Session without a nil guard.
- **Impact:** After creating and persisting ENUM-based schema, restarting the server can crash before it accepts connections, leaving affected databases unavailable until repaired. This blocks normal read/write access for users of those databases.
- **Steps to reproduce:**
1. Start doltgres with a fresh data directory.
2. Create an ENUM type and a table that uses that type, then insert rows.
3. Stop the server and restart using the same data directory.
4. Observe startup panic before ReadyForQuery with stack through recursiveDeserializeType -> GetTypesCollectionFromContext -> getContextValues.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** core/context.go assumes ctx is non-nil in getContextValues (line 56), and core/context.go line 421 calls that helper from GetTypesCollectionFromContext without validating ctx. During startup deserialization, server/types/serialization.go line 267 calls GetTypesCollectionFromContext(ctx, "") when ctx may be nil in this dependency path. The PR diff modified go.mod dependency lines (9, 12, 15) and the panic stack shows the new go-mysql-server and dolt versions on this call chain, making this a regression introduced by changed dependency code paths. Smallest practical fix: add explicit nil-context handling in GetTypesCollectionFromContext/getContextValues and/or avoid calling context-dependent type collection lookup from recursiveDeserializeType when ctx is nil during startup deserialization.
- **Why this is likely a bug:** This is a production-code startup path and the observed panic is a deterministic nil-pointer dereference in repository code, not a harness artifact. A database containing valid persisted custom-type metadata should not make the server crash before it can accept connections.
**Relevant code:**
`go.mod:9-12`
~~~go
github.com/dolthub/dolt/go v0.40.5-0.20260626214637-25552a68b104
github.com/dolthub/go-mysql-server v0.20.1-0.20260626203645-7c333f13eaca
~~~
`core/context.go:55-57`
~~~go
func getContextValues(ctx *sql.Context) (*contextValues, error) {
sess := dsess.DSessFromSess(ctx.Session)
if sess.DoltgresSessObj == nil {
~~~
`core/context.go:420-422`
~~~go
func GetTypesCollectionFromContext(ctx *sql.Context, database string) (*typecollection.TypeCollection, error) {
cv, err := getContextValues(ctx)
if err != nil {
~~~
`server/types/serialization.go:266-268`
~~~go
if typeColl == nil {
typeColl, err = GetTypesCollectionFromContext(ctx, "")
if err != nil {
~~~
| github.com/cockroachdb/apd/v3 v3.2.3 | ||
| github.com/cockroachdb/errors v1.7.5 | ||
| github.com/dolthub/dolt/go v0.40.5-0.20260624213831-feba4a825720 | ||
| github.com/dolthub/dolt/go v0.40.5-0.20260626214637-25552a68b104 |
There was a problem hiding this comment.
Persisted schema loads while referenced custom type fails resolution
What failed: During restart, type deserialization for persisted table schemas executes with a nil *sql.Context and dereferences ctx.Session in core.getContextValues, causing a SIGSEGV and process exit.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: High
- Impact: Servers that restart with persisted tables using custom ENUM or COMPOSITE types can crash on startup, making affected databases unavailable. Users cannot reliably bring the service back up for those datasets until the defect is fixed or the data is changed.
- Steps to Reproduce:
- Start doltgres with a fresh DOLTGRES_DATA_DIR and create custom types plus tables that use those types.
- Insert rows, stop the server, then restart using the same data directory.
- Observe startup panic: stack reaches recursiveDeserializeType -> GetTypesCollectionFromContext(nil) -> getContextValues and crashes before ReadyForQuery.
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: The panic is code-backed in repository paths: core/context.go dereferences ctx.Session without a nil guard, and core/init.go registers GetTypesCollectionFromContext for type resolution used by server/types/serialization.go recursiveDeserializeType. Runtime evidence shows this path is exercised from dependency code during startup table iteration. PR context shows only dependency updates in go.mod/go.sum, with go.mod changed lines bumping github.com/dolthub/dolt/go and github.com/dolthub/go-mysql-server; the stack trace references those bumped module versions and the regression appears when those changed dependency lines are in place.
- Why this is likely a bug: A persisted data directory that previously operated should not make the server crash during restart. The production startup path dereferences a nil context and aborts the process instead of handling unresolved custom-type loading deterministically.
Relevant code
go.mod:9-12
github.com/dolthub/dolt/go v0.40.5-0.20260626214637-25552a68b104
github.com/dolthub/go-mysql-server v0.20.1-0.20260626203645-7c333f13eaca
core/context.go:55-57
func getContextValues(ctx *sql.Context) (*contextValues, error) {
sess := dsess.DSessFromSess(ctx.Session)
if sess.DoltgresSessObj == nil {server/types/serialization.go:266-268
if typeColl == nil {
typeColl, err = GetTypesCollectionFromContext(ctx, "")
if err != nil {core/init.go:40-42
pgtypes.GetTypesCollectionFromContext = func(ctx *sql.Context, database string) (pgtypes.TypeCollection, error) {
return GetTypesCollectionFromContext(ctx, database)
}Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**High severity — Persisted schema loads while referenced custom type fails resolution**
**What failed:** During restart, type deserialization for persisted table schemas executes with a nil *sql.Context and dereferences ctx.Session in core.getContextValues, causing a SIGSEGV and process exit.
- **Impact:** Servers that restart with persisted tables using custom ENUM or COMPOSITE types can crash on startup, making affected databases unavailable. Users cannot reliably bring the service back up for those datasets until the defect is fixed or the data is changed.
- **Steps to reproduce:**
1. Start doltgres with a fresh DOLTGRES_DATA_DIR and create custom types plus tables that use those types.
2. Insert rows, stop the server, then restart using the same data directory.
3. Observe startup panic: stack reaches recursiveDeserializeType -> GetTypesCollectionFromContext(nil) -> getContextValues and crashes before ReadyForQuery.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The panic is code-backed in repository paths: core/context.go dereferences ctx.Session without a nil guard, and core/init.go registers GetTypesCollectionFromContext for type resolution used by server/types/serialization.go recursiveDeserializeType. Runtime evidence shows this path is exercised from dependency code during startup table iteration. PR context shows only dependency updates in go.mod/go.sum, with go.mod changed lines bumping github.com/dolthub/dolt/go and github.com/dolthub/go-mysql-server; the stack trace references those bumped module versions and the regression appears when those changed dependency lines are in place.
- **Why this is likely a bug:** A persisted data directory that previously operated should not make the server crash during restart. The production startup path dereferences a nil context and aborts the process instead of handling unresolved custom-type loading deterministically.
**Relevant code:**
`go.mod:9-12`
~~~gomod
github.com/dolthub/dolt/go v0.40.5-0.20260626214637-25552a68b104
github.com/dolthub/go-mysql-server v0.20.1-0.20260626203645-7c333f13eaca
~~~
`core/context.go:55-57`
~~~go
func getContextValues(ctx *sql.Context) (*contextValues, error) {
sess := dsess.DSessFromSess(ctx.Session)
if sess.DoltgresSessObj == nil {
~~~
`server/types/serialization.go:266-268`
~~~go
if typeColl == nil {
typeColl, err = GetTypesCollectionFromContext(ctx, "")
if err != nil {
~~~
`core/init.go:40-42`
~~~go
pgtypes.GetTypesCollectionFromContext = func(ctx *sql.Context, database string) (pgtypes.TypeCollection, error) {
return GetTypesCollectionFromContext(ctx, database)
}
~~~
Contributor
Footnotes
|
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
2 participants
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.
☕ An Automated Dependency Version Bump PR 👑
Initial Changes
The changes contained in this PR were produced by `go get`ing the dependency.
```bash
go get github.com/dolthub/[dependency]/go@[commit]
```