Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
117710: plpgsql: add support for nested blocks r=DrewKimball a=DrewKimball

#### plpgsql: change order of params and variables

Previously, the parameters of a continuation sub-routine would be chosen
by first appending the variables of the (single) block, then the parameters
of the PL/pgSQL routine. In preparation for supporting nested blocks, this
commit changes the order to first add the routine's parameters, and then the
variables from the block. This prepares to enforce an invariant that the
parameters/arguments of a parent sub-routine are always a prefix of those
of a child sub-routine. This will simplify exception handling later on.

#### plpgsql: allow early exit for visitor

This commit makes two changes to the plpgsql Visitor interface and
implementation - first, the `changed` return value has been replaced
with pointer comparisons. Second, a new return value, `recurse`,
indicates whether a node's children should be visited. This will
allow for Visitor implementations that have to return early.

#### plpgsql: refactor logic for building PL/pgSQL blocks

This commit pulls the logic for handling a PL/pgSQL block into a single
method. Information specific to a given block (including a pointer to the
parent block) is stored in a new struct, `blockScope`, which is allocated
when the block is built. Note that nested blocks are still not allowed in
this commit.

#### plpgsql: add support for nested blocks

This commit makes the final change to support nested blocks with some
limitations, and adds corresponding tests. Currently, variable shadowing
is not allowed, and a block cannot be nested in another block that has an
exception handler.

Fixes #114775

Release note (sql change): PL/pgSQL now supports nested blocks, with
the following limitations for now: variable shadowing is disallowed,
and exception handlers cannot be used in a routine with nested blocks.

118298: ttljob: mark errors as retryable when possible r=rafiss a=rafiss

fixes #110823
fixes #117985
Release note: None

118595: roachtest: log when tests use spot VMs r=srosenberg a=renatolabs

It's currently not easy to determine which tests used spot VMs in a nightly run. This commit updates the test runner so that, when we decide to run a test with spot VMs, we log that decision. This should make it trivial to check which  tests ran on spot VMs in any given run and compare that to the failures observed to get a sense of the failure ratio.

Epic: none

Release note: None

Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Renato Costa <renato@cockroachlabs.com>
  • Loading branch information
4 people committed Feb 1, 2024
4 parents 7b66dc4 + 082a198 + 6f9c500 + 33a228a commit 5d6c7e6
Show file tree
Hide file tree
Showing 29 changed files with 1,921 additions and 781 deletions.
479 changes: 479 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/plpgsql_block

Large diffs are not rendered by default.

146 changes: 97 additions & 49 deletions pkg/ccl/logictestccl/testdata/logic_test/udf_rewrite

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/fakedist-disk/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 17,
shard_count = 18,
tags = [
"ccl_test",
"cpu:2",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/fakedist-disk/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/fakedist-vec-off/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 17,
shard_count = 18,
tags = [
"ccl_test",
"cpu:2",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/fakedist-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/fakedist/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 18,
shard_count = 19,
tags = [
"ccl_test",
"cpu:2",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/fakedist/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 17,
shard_count = 18,
tags = [
"ccl_test",
"cpu:1",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/local-mixed-23.2/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 17,
shard_count = 18,
tags = [
"ccl_test",
"cpu:1",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local-mixed-23.2/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/local-vec-off/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 17,
shard_count = 18,
tags = [
"ccl_test",
"cpu:1",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/tests/local/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_test(
"//build/toolchains:is_heavy": {"Pool": "heavy"},
"//conditions:default": {"Pool": "large"},
}),
shard_count = 32,
shard_count = 33,
tags = [
"ccl_test",
"cpu:1",
Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/cmd/roachtest/test_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ func (r *testRunner) runWorker(

// TODO(babusrithar): remove this once we see enough data in nightly runs. This is a temp logic to test spot VMs.
if roachtestflags.Cloud == spec.GCE && testToRun.spec.Benchmark && rand.Float64() <= 0.5 {
l.PrintfCtx(ctx, "using spot VMs to run test %s", testToRun.spec.Name)
testToRun.spec.Cluster.UseSpotVMs = true
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/opt/optbuilder/create_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,10 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o
// volatility of stable functions. If folded, we only get a scalar and lose
// the volatility.
b.factory.FoldingControl().TemporarilyDisallowStableFolds(func() {
var plBuilder plpgsqlBuilder
plBuilder.init(b, nil /* colRefs */, paramTypes, stmt.AST, funcReturnType)
stmtScope = plBuilder.build(stmt.AST, bodyScope)
plBuilder := newPLpgSQLBuilder(
b, cf.Name.Object(), nil /* colRefs */, paramTypes, funcReturnType,
)
stmtScope = plBuilder.buildBlock(stmt.AST, bodyScope)
})
checkStmtVolatility(targetVolatility, stmtScope, stmt)

Expand Down

0 comments on commit 5d6c7e6

Please sign in to comment.