Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
108217: opt: simplify ScalarBuilder.Build r=mgartner a=mgartner

Epic: None

Release note: None

108315: release: bump CRDB orchestrator configurations to v23.1.8 r=Xiang-Gu a=Xiang-Gu

Epic: None
Release note: None

108318: release: update predecessor map for 23.1.8 r=Xiang-Gu a=Xiang-Gu

Release note: None
Epic: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
  • Loading branch information
3 people committed Aug 7, 2023
4 parents 79a09a2 + 806f28b + 6e0f028 + 28318f2 commit 782a301
Show file tree
Hide file tree
Showing 36 changed files with 51 additions and 58 deletions.
2 changes: 1 addition & 1 deletion cloud/kubernetes/bring-your-own-certs/client.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
# Keep a pod open indefinitely so kubectl exec can be used to get a shell to it
# and run cockroach client commands, such as cockroach sql, cockroach node status, etc.
command:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cluster-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
command:
- "/cockroach/cockroach"
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cockroachdb-statefulset-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cockroachdb-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/multiregion/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/multiregion/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ spec:
name: cockroach-env
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ spec:
hostNetwork: true
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: If you configured taints to give CockroachDB exclusive access to nodes, feel free
# to remove the requests and limits sections. If you didn't, you'll need to change these to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: If you configured taints to give CockroachDB exclusive access to nodes, feel free
# to remove the requests and limits sections. If you didn't, you'll need to change these to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ spec:
- name: cockroachdb
# NOTE: Always use the most recent version of CockroachDB for the best
# performance and reliability.
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ spec:
- name: cockroachdb
# NOTE: Always use the most recent version of CockroachDB for the best
# performance and reliability.
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cluster-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
command:
- "/cockroach/cockroach"
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cluster-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
command:
- "/cockroach/cockroach"
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v23.1.7
image: cockroachdb/cockroach:v23.1.8
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/opt/idxconstraint/index_constraints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ func TestIndexConstraints(t *testing.T) {
computedCols = make(map[opt.ColumnID]opt.ScalarExpr)
for col, expr := range sv.ComputedCols() {
b := optbuilder.NewScalar(context.Background(), &semaCtx, &evalCtx, &f)
if err := b.Build(expr); err != nil {
computedColExpr, err := b.Build(expr)
if err != nil {
d.Fatalf(t, "error building computed column expression: %v", err)
}
computedColExpr := f.Memo().RootExpr().(opt.ScalarExpr)
computedCols[col] = computedColExpr
var sharedProps props.Shared
memo.BuildSharedProps(computedColExpr, &sharedProps, &evalCtx)
Expand Down Expand Up @@ -314,10 +314,10 @@ func buildFilters(
return memo.FiltersExpr{}, err
}
b := optbuilder.NewScalar(context.Background(), semaCtx, evalCtx, f)
if err := b.Build(expr); err != nil {
root, err := b.Build(expr)
if err != nil {
return memo.FiltersExpr{}, err
}
root := f.Memo().RootExpr().(opt.ScalarExpr)
if _, ok := root.(*memo.TrueExpr); ok {
return memo.TrueFilter, nil
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/sql/opt/lookupjoin/constraint_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ func TestLookupConstraints(t *testing.T) {
return 0, opt.ColSet{}, err
}
b := optbuilder.NewScalar(context.Background(), &semaCtx, &evalCtx, &f)
if err := b.Build(expr); err != nil {
compExpr, err := b.Build(expr)
if err != nil {
return 0, opt.ColSet{}, err
}
compExpr := f.Memo().RootExpr().(opt.ScalarExpr)
var sharedProps props.Shared
memo.BuildSharedProps(compExpr, &sharedProps, &evalCtx)
md.TableMeta(tableID).AddComputedCol(colID, compExpr, sharedProps.OuterCols)
Expand Down Expand Up @@ -312,12 +312,11 @@ func makeFiltersExpr(
}

b := optbuilder.NewScalar(context.Background(), semaCtx, evalCtx, f)
if err := b.Build(expr); err != nil {
root, err := b.Build(expr)
if err != nil {
return nil, err
}

root := f.Memo().RootExpr().(opt.ScalarExpr)

return memo.FiltersExpr{f.ConstructFiltersItem(root)}, nil
}

Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/opt/memo/expr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/opt"
"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
"github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder"
"github.com/cockroachdb/cockroach/pkg/sql/opt/testutils"
Expand Down Expand Up @@ -93,11 +92,11 @@ func TestExprIsNeverNull(t *testing.T) {
}

b := optbuilder.NewScalar(ctx, &semaCtx, &evalCtx, o.Factory())
err = b.Build(expr)
scalar, err := b.Build(expr)
if err != nil {
return fmt.Sprintf("error: %s\n", strings.TrimSpace(err.Error()))
}
result := memo.ExprIsNeverNull(o.Memo().RootExpr().(opt.ScalarExpr), sv.NotNullCols())
result := memo.ExprIsNeverNull(scalar, sv.NotNullCols())
return fmt.Sprintf("%t\n", result)

default:
Expand Down
6 changes: 0 additions & 6 deletions pkg/sql/opt/memo/memo.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,6 @@ func (m *Memo) SetRoot(e RelExpr, phys *physical.Required) {
}
}

// SetScalarRoot stores the root memo expression when it is a scalar expression.
// Used only for testing.
func (m *Memo) SetScalarRoot(scalar opt.ScalarExpr) {
m.rootExpr = scalar
}

// HasPlaceholders returns true if the memo contains at least one placeholder
// operator.
func (m *Memo) HasPlaceholders() bool {
Expand Down
5 changes: 3 additions & 2 deletions pkg/sql/opt/memo/memo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ func TestCompositeSensitive(t *testing.T) {
}

b := optbuilder.NewScalar(context.Background(), &semaCtx, &evalCtx, &f)
if err := b.Build(expr); err != nil {
scalar, err := b.Build(expr)
if err != nil {
d.Fatalf(t, "error building: %v", err)
}
return fmt.Sprintf("%v", memo.CanBeCompositeSensitive(md, f.Memo().RootExpr()))
return fmt.Sprintf("%v", memo.CanBeCompositeSensitive(md, scalar))
})
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/optbuilder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ func TestBuilder(t *testing.T) {
// of the build process.
o.DisableOptimizations()
b := optbuilder.NewScalar(ctx, &semaCtx, &evalCtx, o.Factory())
err = b.Build(expr)
scalar, err := b.Build(expr)
if err != nil {
return fmt.Sprintf("error: %s\n", strings.TrimSpace(err.Error()))
}
f := memo.MakeExprFmtCtx(
ctx, tester.Flags.ExprFormat, false /* redactableValues */, o.Memo(), catalog,
)
f.FormatExpr(o.Memo().RootExpr())
f.FormatExpr(scalar)
return f.Buffer.String()

default:
Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/opt/optbuilder/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ func NewScalar(

// Build a memo structure from a TypedExpr: the root group represents a scalar
// expression equivalent to expr.
func (sb *ScalarBuilder) Build(expr tree.Expr) (err error) {
func (sb *ScalarBuilder) Build(expr tree.Expr) (_ opt.ScalarExpr, err error) {
defer func() {
if r := recover(); r != nil {
// This code allows us to propagate errors without adding lots of checks
Expand All @@ -1194,8 +1194,7 @@ func (sb *ScalarBuilder) Build(expr tree.Expr) (err error) {

typedExpr := sb.scope.resolveType(expr, types.Any)
scalar := sb.buildScalar(typedExpr, &sb.scope, nil, nil, nil)
sb.factory.Memo().SetScalarRoot(scalar)
return nil
return scalar, nil
}

// reType is similar to tree.ReType, except that it panics with an internal
Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/opt/partialidx/implicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,10 @@ func makeFiltersExpr(
}

b := optbuilder.NewScalar(context.Background(), semaCtx, evalCtx, f)
if err := b.Build(expr); err != nil {
root, err := b.Build(expr)
if err != nil {
return nil, err
}

root := f.Memo().RootExpr().(opt.ScalarExpr)

return memo.FiltersExpr{f.ConstructFiltersItem(root)}, nil
}
5 changes: 3 additions & 2 deletions pkg/sql/opt/testutils/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,12 @@ func BuildScalar(
}

b := optbuilder.NewScalar(context.Background(), semaCtx, evalCtx, f)
if err := b.Build(expr); err != nil {
root, err := b.Build(expr)
if err != nil {
t.Fatal(err)
}

return f.Memo().RootExpr().(opt.ScalarExpr)
return root
}

// BuildFilters builds the given input string as a FiltersExpr and returns it.
Expand Down
Loading

0 comments on commit 782a301

Please sign in to comment.