Skip to content

Commit f6947ce

Browse files
committed
sql/schemachanger: add decoder for schemaChangerUserError
Previously, if a schemaChangerUserError was sent over the network during a backfill, it was not properly recognized on the receiving node. Without a decoder, the error would be an unresolved opaqueWrapper. As a result, the HasSchemaChangerUserError function, which confirms the error by checking its type, would not work correctly. This patch adds a decoder for this error type, allowing it to be decoded properly on remote nodes. Fixes: #141352 Release note: None
1 parent cd43ab3 commit f6947ce

File tree

5 files changed

+57
-1
lines changed

5 files changed

+57
-1
lines changed

pkg/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ ALL_TESTS = [
598598
"//pkg/sql/schemachanger/scbuild/internal/scbuildstmt:scbuildstmt_test",
599599
"//pkg/sql/schemachanger/scbuild:scbuild_test",
600600
"//pkg/sql/schemachanger/scdecomp:scdecomp_test",
601+
"//pkg/sql/schemachanger/scerrors:scerrors_test",
601602
"//pkg/sql/schemachanger/scexec/backfiller:backfiller_test",
602603
"//pkg/sql/schemachanger/scexec:scexec_test",
603604
"//pkg/sql/schemachanger/scpb:scpb_test",
@@ -2239,6 +2240,7 @@ GO_TARGETS = [
22392240
"//pkg/sql/schemachanger/scdeps/sctestutils:sctestutils",
22402241
"//pkg/sql/schemachanger/scdeps:scdeps",
22412242
"//pkg/sql/schemachanger/scerrors:scerrors",
2243+
"//pkg/sql/schemachanger/scerrors:scerrors_test",
22422244
"//pkg/sql/schemachanger/scexec/backfiller:backfiller",
22432245
"//pkg/sql/schemachanger/scexec/backfiller:backfiller_test",
22442246
"//pkg/sql/schemachanger/scexec/scmutationexec:scmutationexec",

pkg/sql/schemachanger/scerrors/BUILD.bazel

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@io_bazel_rules_go//go:def.bzl", "go_library")
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "scerrors",
@@ -14,5 +14,17 @@ go_library(
1414
"//pkg/util/timeutil",
1515
"@com_github_cockroachdb_errors//:errors",
1616
"@com_github_cockroachdb_redact//:redact",
17+
"@com_github_gogo_protobuf//proto",
18+
],
19+
)
20+
21+
go_test(
22+
name = "scerrors_test",
23+
srcs = ["errors_test.go"],
24+
deps = [
25+
":scerrors",
26+
"@com_github_cockroachdb_errors//:errors",
27+
"@com_github_cockroachdb_errors//errbase",
28+
"@com_github_stretchr_testify//require",
1729
],
1830
)

pkg/sql/schemachanger/scerrors/errors.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
2020
"github.com/cockroachdb/errors"
2121
"github.com/cockroachdb/redact"
22+
"github.com/gogo/protobuf/proto"
2223
)
2324

2425
// EventLogger is a convenience object used for logging schema changer events.
@@ -206,12 +207,26 @@ func (e *schemaChangerUserError) SafeFormatError(p errors.Printer) (next error)
206207
return e.err
207208
}
208209

210+
// Error implements error.
209211
func (e *schemaChangerUserError) Error() string {
210212
// We don't want to print the schemaChangerUserError wrapper in the error,
211213
// this only serves as a marker to the declarative schema changer to surface.
212214
return fmt.Sprintf("%v", e.err)
213215
}
214216

217+
// Unwrap implements errors.Wrapper.
215218
func (e *schemaChangerUserError) Unwrap() error {
216219
return e.err
217220
}
221+
222+
// schemaChangerUserErrorDecodeWrapper is a wrapper decoder for
223+
// schemaChangerUserError.
224+
func schemaChangerUserErrorDecodeWrapper(
225+
_ context.Context, cause error, _ string, _ []string, payload proto.Message,
226+
) error {
227+
return &schemaChangerUserError{err: cause}
228+
}
229+
230+
func init() {
231+
errors.RegisterWrapperDecoder(errors.GetTypeKey((*schemaChangerUserError)(nil)), schemaChangerUserErrorDecodeWrapper)
232+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package scerrors_test
7+
8+
import (
9+
"context"
10+
"testing"
11+
12+
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
13+
"github.com/cockroachdb/errors"
14+
"github.com/cockroachdb/errors/errbase"
15+
"github.com/stretchr/testify/require"
16+
)
17+
18+
// TestEncodeSchemaChangeUserError tests that the error is encoded and decoded
19+
// correctly if sent over the network.
20+
func TestEncodeSchemaChangeUserError(t *testing.T) {
21+
ctx := context.Background()
22+
base := scerrors.SchemaChangerUserError(errors.New("boom"))
23+
err := errbase.EncodeError(ctx, base)
24+
decodeError := errbase.DecodeError(ctx, err)
25+
require.True(t, scerrors.HasSchemaChangerUserError(decodeError))
26+
}

pkg/testutils/lint/lint_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,6 +1583,7 @@ func TestLint(t *testing.T) {
15831583
":!sql/plpgsql/plpgsql_error.go",
15841584
":!sql/protoreflect/redact.go",
15851585
":!sql/colexecerror/error.go",
1586+
":!sql/schemachanger/scerrors/errors.go",
15861587
":!util/timeutil/timeout_error.go",
15871588
":!util/protoutil/jsonpb_marshal.go",
15881589
":!util/protoutil/marshal.go",

0 commit comments

Comments
 (0)