From 9e847447cd461047b2ab88ee92ac122784a5dd81 Mon Sep 17 00:00:00 2001 From: Steven Danna Date: Thu, 22 Dec 2022 20:01:20 +0000 Subject: [PATCH 1/5] streamingccl: don't resume job in TestTenantStreamingCutoverOnSourceFailure ALTER TENANT ... COMPLETE REPLICATION resumes the job for the user, so there is no need to resume the job here. This does raise the question about whether or not it is the right behaviour to resume the job by default. Fixes #94034 Release note: None --- .../streamingccl/streamingest/replication_stream_e2e_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/ccl/streamingccl/streamingest/replication_stream_e2e_test.go b/pkg/ccl/streamingccl/streamingest/replication_stream_e2e_test.go index aaa2ac69ab6f..110c5f3afc21 100644 --- a/pkg/ccl/streamingccl/streamingest/replication_stream_e2e_test.go +++ b/pkg/ccl/streamingccl/streamingest/replication_stream_e2e_test.go @@ -583,9 +583,6 @@ func TestTenantStreamingCutoverOnSourceFailure(t *testing.T) { c.DestSysSQL.Exec(c.T, `ALTER TENANT $1 COMPLETE REPLICATION TO SYSTEM TIME $2::string`, c.Args.DestTenantName, cutoverTime.AsOfSystemTime()) - // Resume ingestion. - c.DestSysSQL.Exec(t, fmt.Sprintf("RESUME JOB %d", ingestionJobID)) - // Ingestion job should succeed despite source failure due to the successful cutover jobutils.WaitForJobToSucceed(t, c.DestSysSQL, jobspb.JobID(ingestionJobID)) } From f907b7dd330ef6e18a1223acc980cb333974cec0 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Tue, 24 Jan 2023 12:41:52 -0500 Subject: [PATCH 2/5] multiregionccl: add a missing log scope Epic: none Release note: None --- pkg/ccl/multiregionccl/regional_by_row_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/ccl/multiregionccl/regional_by_row_test.go b/pkg/ccl/multiregionccl/regional_by_row_test.go index 4007fe944594..8940b117bb23 100644 --- a/pkg/ccl/multiregionccl/regional_by_row_test.go +++ b/pkg/ccl/multiregionccl/regional_by_row_test.go @@ -799,6 +799,7 @@ USE t; for _, rbrChange := range regionalByRowChanges { for _, regionChange := range regionChanges { t.Run(fmt.Sprintf("setup %s executing %s with racing %s", rbrChange.setup, rbrChange.cmd, regionChange.cmd), func(t *testing.T) { + defer log.Scope(t).Close(t) interruptStartCh := make(chan struct{}) interruptEndCh := make(chan struct{}) performInterrupt := false From 4d4539fbad5a5d9bc66e390a69cd1016425752f7 Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Tue, 24 Jan 2023 10:54:13 -0500 Subject: [PATCH 3/5] roachtest: fix env var passing in activerecord test This patch fixes the rails version pinning in the activerecord roachtest. The rails version is passed in via the env variable `RAILS_VERSION` and was previously being set before the `sudo` in the adapter install command and thus erroneously discarded. Release note: None --- pkg/cmd/roachtest/tests/activerecord.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/roachtest/tests/activerecord.go b/pkg/cmd/roachtest/tests/activerecord.go index 30b083557cb3..d1084b69c62b 100644 --- a/pkg/cmd/roachtest/tests/activerecord.go +++ b/pkg/cmd/roachtest/tests/activerecord.go @@ -159,7 +159,7 @@ func registerActiveRecord(r registry.Registry) { "installing gems", fmt.Sprintf( `cd /mnt/data1/activerecord-cockroachdb-adapter/ && `+ - `RAILS_VERSION=%s sudo bundle install`, supportedRailsVersion), + `sudo RAILS_VERSION=%s bundle install`, supportedRailsVersion), ); err != nil { t.Fatal(err) } @@ -172,8 +172,9 @@ func registerActiveRecord(r registry.Registry) { t.Status("running activerecord test suite") result, err := c.RunWithDetailsSingleNode(ctx, t.L(), node, - `cd /mnt/data1/activerecord-cockroachdb-adapter/ && `+ - `sudo RUBYOPT="-W0" TESTOPTS="-v" bundle exec rake test`, + fmt.Sprintf( + `cd /mnt/data1/activerecord-cockroachdb-adapter/ && `+ + `sudo RAILS_VERSION=%s RUBYOPT="-W0" TESTOPTS="-v" bundle exec rake test`, supportedRailsVersion), ) // Fatal for a roachprod or SSH error. A roachprod error is when result.Err==nil. From eb882695d505de9cea71a155dddcc0fd40c376c4 Mon Sep 17 00:00:00 2001 From: Mark Sirek Date: Tue, 24 Jan 2023 18:11:37 -0800 Subject: [PATCH 4/5] builtins: array_to_string should traverse nested arrays Fixes #95588 In Postgres, `array_to_string` traverses nested arrays and prints their contents. In CRDB, the nested array structures are printed out. For example, `SELECT array_to_string(ARRAY[ARRAY[ARRAY[5,6], ARRAY[2,3]]], ' ');` CRDB Result: `ARRAY[ARRAY[5:::INT8,6:::INT8],ARRAY[2:::INT8,3:::INT8]]` Postgres Result: `5 6 2 3` This fix brings the behavior of `array_to_string` in line with Postgres, and avoids printing the nested ARRAY structures. Some tools like GoldenGate rely on Postgres-compatible behavior of `array_to_string` for proper functioning. Release note (bug fix): This patch fixes the array_to_string built-in function so that nested arrays are traversed without printing 'ARRAY' at each nesting level. --- .../testdata/logic_test/builtin_function | 12 ++++++++++++ pkg/sql/sem/builtins/builtins.go | 15 +++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index 0d5cd8c50020..c9750ebdcd18 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/pkg/sql/logictest/testdata/logic_test/builtin_function @@ -2517,6 +2517,18 @@ SELECT array_to_string(NULL, ','), array_to_string(NULL, 'foo', 'zerp') ---- NULL NULL +# Builtin array_to_string should recursively search nested arrays. +query T +SELECT array_to_string(ARRAY[ARRAY[ARRAY[5,6], ARRAY[2,3]], ARRAY[ARRAY[7,8], ARRAY[4,44]]], ' '); +---- +5 6 2 3 7 8 4 44 + +# Builtin array_to_string should recursively search nested arrays. +query T +SELECT array_to_string(ARRAY[(SELECT ARRAY[1,2]::int2vector)],' '); +---- +1 2 + # Examples from https://www.postgresql.org/docs/9.3/functions-string.html#FUNCTIONS-STRING-FORMAT query T SELECT format('Hello %s', 'World') diff --git a/pkg/sql/sem/builtins/builtins.go b/pkg/sql/sem/builtins/builtins.go index acb682d1e16c..e1f33fd7f40d 100644 --- a/pkg/sql/sem/builtins/builtins.go +++ b/pkg/sql/sem/builtins/builtins.go @@ -9857,6 +9857,13 @@ func arrayToString( evalCtx *eval.Context, arr *tree.DArray, delim string, nullStr *string, ) (tree.Datum, error) { f := evalCtx.FmtCtx(tree.FmtArrayToString) + arrayToStringHelper(evalCtx, arr, delim, nullStr, f) + return tree.NewDString(f.CloseAndGetString()), nil +} + +func arrayToStringHelper( + evalCtx *eval.Context, arr *tree.DArray, delim string, nullStr *string, f *tree.FmtCtx, +) { for i := range arr.Array { if arr.Array[i] == tree.DNull { @@ -9865,13 +9872,17 @@ func arrayToString( } f.WriteString(*nullStr) } else { - f.FormatNode(arr.Array[i]) + if nestedArray, ok := arr.Array[i].(*tree.DArray); ok { + // "Unpack" nested arrays to be consistent with postgres. + arrayToStringHelper(evalCtx, nestedArray, delim, nullStr, f) + } else { + f.FormatNode(arr.Array[i]) + } } if i < len(arr.Array)-1 { f.WriteString(delim) } } - return tree.NewDString(f.CloseAndGetString()), nil } // encodeEscape implements the encode(..., 'escape') Postgres builtin. It's From 3386c7231ac095a73a3632c4ef2a07f686e929cc Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Wed, 25 Jan 2023 14:04:53 +0000 Subject: [PATCH 5/5] sql/execinfrapb: remove no-effect nullable from GenerativeSplitAndScatterSpec Remove no-effect nullable from GenerativeSplitAndScatterSpec that was causing warning messages during build. Release note: None --- pkg/sql/execinfrapb/processors_bulk_io.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sql/execinfrapb/processors_bulk_io.proto b/pkg/sql/execinfrapb/processors_bulk_io.proto index 0eb6c97c0fd4..2a340b433448 100644 --- a/pkg/sql/execinfrapb/processors_bulk_io.proto +++ b/pkg/sql/execinfrapb/processors_bulk_io.proto @@ -407,7 +407,7 @@ message GenerativeSplitAndScatterSpec { repeated roachpb.Span spans = 10 [(gogoproto.nullable) = false]; repeated jobs.jobspb.RestoreDetails.BackupLocalityInfo backup_locality_info = 11 [(gogoproto.nullable) = false]; // HighWater is the high watermark of the previous run of restore. - optional bytes high_water = 12 [(gogoproto.nullable) = false]; + optional bytes high_water = 12; // User who initiated the restore. optional string user_proto = 13 [(gogoproto.nullable) = false, (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/security/username.SQLUsernameProto"]; // ChunkSize is the number of import spans per chunk.