Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
89874: cmdccl: remove `enc_utils` r=jbowens a=nicktrav

Remove an obsolete, broken command. The functionality has been superseded by #89095 and #89873.

Touches #89095.
Epic: None.
Release note: None.

89905: multitenant: fix failing distsql test r=cucaroach a=cucaroach

Test was erroneously expecting a bundle in the bundle=off config.

Fixes: #89720

Release note: None


89910: sql: `tree.ParseJSON` should not ignore trailing data r=miretskiy a=miretskiy

`tree.ParseJSON` uses `Decoder.More()` method to determine if the input contains trailing data.  The implementation made incorrect assumptions as to the reason why `Decoder.More()` allows input to contain `]` or `}` characters, even when JSON object has been consumed.

This PR fixes and comments those faulty assumptions, and fixes multiple existing tests that seemed to rely on those faulty assumptions.

In particular, prior to this PR, the following input would be allowed (i.e. extra end of object character `}`):
```
[email protected]:26257/movr> select '{"longKey1":"longValue1"}}'::jsonb;
            jsonb
------------------------------
  {"longKey1": "longValue1"}
(1 row)
```

But so would the following be allowed:
```
[email protected]:26257/movr> select '{"longKey1":"longValue1"}} should this data be ignored?'::jsonb;
            jsonb
------------------------------
  {"longKey1": "longValue1"}
(1 row)
```

So, the  issue is: if above conversion was executed to insert JSONB into the database, we would silently
truncate (corrupt) JSON input, and instead of returning an error, we would return success.

This behavior is wrong, and this PR fixes it so that an error is returned:
```
select '{"longKey1":"longValue1"}} should this data be ignored?'::jsonb;
ERROR: could not parse JSON: trailing characters after JSON document
SQLSTATE: 22P02
```

Release note (bug fix): Do not silently truncate trailing characters when attempting to convert corrupt JSON string input into JSONb.
Release note (backward-incompatible change): This change may be backward incompatible to the applications
that previously might have been able to insert corrupt JSON data, but now will receive an error.

Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Yevgeniy Miretskiy <[email protected]>
  • Loading branch information
4 people committed Oct 13, 2022
4 parents 3bdca7d + 5c124a4 + 437e82a + f2d5001 commit 95733e7
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 330 deletions.
3 changes: 0 additions & 3 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -704,8 +704,6 @@ GO_TARGETS = [
"//pkg/ccl/cloudccl/cloudprivilege:cloudprivilege_test",
"//pkg/ccl/cloudccl/externalconn:externalconn_test",
"//pkg/ccl/cloudccl/gcp:gcp_test",
"//pkg/ccl/cmdccl/enc_utils:enc_utils",
"//pkg/ccl/cmdccl/enc_utils:enc_utils_lib",
"//pkg/ccl/cmdccl/stub-schema-registry:stub-schema-registry",
"//pkg/ccl/cmdccl/stub-schema-registry:stub-schema-registry_lib",
"//pkg/ccl/gssapiccl:gssapiccl",
Expand Down Expand Up @@ -2190,7 +2188,6 @@ GET_X_DATA_TARGETS = [
"//pkg/ccl/cloudccl/cloudprivilege:get_x_data",
"//pkg/ccl/cloudccl/externalconn:get_x_data",
"//pkg/ccl/cloudccl/gcp:get_x_data",
"//pkg/ccl/cmdccl/enc_utils:get_x_data",
"//pkg/ccl/cmdccl/stub-schema-registry:get_x_data",
"//pkg/ccl/gssapiccl:get_x_data",
"//pkg/ccl/importerccl:get_x_data",
Expand Down
1 change: 0 additions & 1 deletion pkg/ccl/cmdccl/enc_utils/.gitignore

This file was deleted.

24 changes: 0 additions & 24 deletions pkg/ccl/cmdccl/enc_utils/BUILD.bazel

This file was deleted.

25 changes: 0 additions & 25 deletions pkg/ccl/cmdccl/enc_utils/README.md

This file was deleted.

248 changes: 0 additions & 248 deletions pkg/ccl/cmdccl/enc_utils/main.go

This file was deleted.

38 changes: 20 additions & 18 deletions pkg/cmd/roachtest/tests/multitenant_distsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,27 +217,29 @@ func runMultiTenantDistSQL(
require.NoError(t, err)
t.L().Printf(str)
}
// Open bundle and verify its contents

sqlConnCtx := clisqlclient.Context{}
conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, instance1.pgURL)
bundles, err := clisqlclient.StmtDiagListBundles(ctx, conn)
require.NoError(t, err)
if bundle {
// Open bundle and verify its contents
sqlConnCtx := clisqlclient.Context{}
conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, instance1.pgURL)
bundles, err := clisqlclient.StmtDiagListBundles(ctx, conn)
require.NoError(t, err)

err = clisqlclient.StmtDiagDownloadBundle(ctx, conn, bundles[len(bundles)-1].ID, "bundle.zip")
require.NoError(t, err)
err = clisqlclient.StmtDiagDownloadBundle(ctx, conn, bundles[len(bundles)-1].ID, "bundle.zip")
require.NoError(t, err)

read, err := zip.OpenReader("bundle.zip")
require.NoError(t, err)
defer func() { _ = read.Close() }()
required := []string{"plan.txt", "statement.sql", "env.sql", "schema.sql", "stats-defaultdb.public.t.sql"}
zipNames := make(map[string]struct{})
for _, f := range read.File {
zipNames[f.Name] = struct{}{}
}
for _, i := range required {
_, found := zipNames[i]
require.True(t, found, "%s not found in bundle", i)
read, err := zip.OpenReader("bundle.zip")
require.NoError(t, err)
defer func() { _ = read.Close() }()
required := []string{"plan.txt", "statement.sql", "env.sql", "schema.sql", "stats-defaultdb.public.t.sql"}
zipNames := make(map[string]struct{})
for _, f := range read.File {
zipNames[f.Name] = struct{}{}
}
for _, i := range required {
_, found := zipNames[i]
require.True(t, found, "%s not found in bundle", i)
}
}
}
}
Loading

0 comments on commit 95733e7

Please sign in to comment.