Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
49307: sql: add support for virtual schema qualified types r=rohany a=rohany

Fixes #16395.

Release note (sql change): Allows for referencing static data types
under the `pg_catalog` qualification like `pg_catalog.int`.

49530: sql: support max/min aggregates on collated strings r=rohany a=rohany

Fixes #45846.
Fixes #46685.

This PR allows for performing the max/min aggregations on collated
strings. Currently, our aggregates would not support container types
like `AnyCollatedString`. This PR extends the aggregates infrastructure
in DistSQL that caused the limitation to allow for container types,
which unblocks #48370.

Release note (sql change): Support the max/min aggregations on collated
strings.

49567: cloud: bump version to 20.1.1 r=dt a=dt

Release note: none.

49574: roachtest: get bigger machines for alterpk-tpcc-250 r=yuzefovich a=yuzefovich

The test has been failing for a while now because we're operating very
close to memory limit. Usually, 3.3.2.6 expensive check query ends up
crashing the node, sometimes it hits SQL memory limit. I've been staring
at it for a while, but there is a discrepancy between the memory usage
reported in runtime stats in logs and the usage that shows up in heap
profiles (for example, I could see 8GB in the former and 3GB in the
latter), and I couldn't figure out where that gaps goes. I don't think
spending any more time on this issue is worthwhile, so this commit bumps
the machine type from cpu-16 to cpu-32 which doubles available RAM from
14.4 to 28.8.

Fixes: #48428.

Release note: None

Co-authored-by: Rohan Yadav <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
4 people committed May 27, 2020
5 parents 8417124 + fbe54be + 974be22 + e583a20 + 1b3e0d7 commit 126df91
Show file tree
Hide file tree
Showing 46 changed files with 663 additions and 395 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 @@ -19,7 +19,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
# 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 @@ -152,7 +152,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
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 @@ -31,7 +31,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -33,7 +33,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -9,7 +9,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -194,7 +194,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the amount of allocatable resources 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 @@ -97,7 +97,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the amount of allocatable resources 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 @@ -8,7 +8,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -10,7 +10,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ spec:
hostNetwork: true
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -197,7 +197,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -140,7 +140,7 @@ spec:
- name: cockroachdb
# NOTE: Always use the most recent version of CockroachDB for the best
# performance and reliability.
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the amount of allocatable resources on each of your Kubernetes nodes by running:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ spec:
- name: cockroachdb
# NOTE: Always use the most recent version of CockroachDB for the best
# performance and reliability.
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the amount of allocatable resources 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 @@ -31,7 +31,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -33,7 +33,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -9,7 +9,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -177,7 +177,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -80,7 +80,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -31,7 +31,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -33,7 +33,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -9,7 +9,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -189,7 +189,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
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 @@ -92,7 +92,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v20.1.0
image: cockroachdb/cockroach:v20.1.1
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
4 changes: 4 additions & 0 deletions docs/generated/sql/aggregates.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@
</span></td></tr>
<tr><td><a name="max"></a><code>max(arg1: <a href="uuid.html">uuid</a>) &rarr; <a href="uuid.html">uuid</a></code></td><td><span class="funcdesc"><p>Identifies the maximum selected value.</p>
</span></td></tr>
<tr><td><a name="max"></a><code>max(arg1: collatedstring{*}) &rarr; anyelement</code></td><td><span class="funcdesc"><p>Identifies the maximum selected value.</p>
</span></td></tr>
<tr><td><a name="max"></a><code>max(arg1: geography) &rarr; geography</code></td><td><span class="funcdesc"><p>Identifies the maximum selected value.</p>
</span></td></tr>
<tr><td><a name="max"></a><code>max(arg1: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Identifies the maximum selected value.</p>
Expand Down Expand Up @@ -143,6 +145,8 @@
</span></td></tr>
<tr><td><a name="min"></a><code>min(arg1: <a href="uuid.html">uuid</a>) &rarr; <a href="uuid.html">uuid</a></code></td><td><span class="funcdesc"><p>Identifies the minimum selected value.</p>
</span></td></tr>
<tr><td><a name="min"></a><code>min(arg1: collatedstring{*}) &rarr; anyelement</code></td><td><span class="funcdesc"><p>Identifies the minimum selected value.</p>
</span></td></tr>
<tr><td><a name="min"></a><code>min(arg1: geography) &rarr; geography</code></td><td><span class="funcdesc"><p>Identifies the minimum selected value.</p>
</span></td></tr>
<tr><td><a name="min"></a><code>min(arg1: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Identifies the minimum selected value.</p>
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/alterpk.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func registerAlterPK(r *testRegistry) {
// Use a 4 node cluster -- 3 nodes will run cockroach, and the last will be the
// workload driver node.
MinVersion: "v20.1.0",
Cluster: makeClusterSpec(4, cpu(16)),
Cluster: makeClusterSpec(4, cpu(32)),
Run: func(ctx context.Context, t *test, c *cluster) {
runAlterPKTPCC(ctx, t, c, 250 /* warehouses */, true /* expensiveChecks */)
},
Expand Down
52 changes: 13 additions & 39 deletions pkg/sql/catalog/accessors/logical_schema_accessors.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/errors"
)

// This file provides reference implementations of the schema accessor
Expand All @@ -42,8 +41,6 @@ func NewLogicalAccessor(
type LogicalSchemaAccessor struct {
catalog.Accessor
vs catalog.VirtualSchemas
// Used to avoid allocations.
tn tree.TableName
}

var _ catalog.Accessor = &LogicalSchemaAccessor{}
Expand Down Expand Up @@ -96,43 +93,20 @@ func (l *LogicalSchemaAccessor) GetObjectDesc(
db, schema, object string,
flags tree.ObjectLookupFlags,
) (catalog.Descriptor, error) {
switch flags.DesiredObjectKind {
case tree.TypeObject:
// TODO(ajwerner): Change this function if we ever expose non-table objects
// underneath virtual schemas. For now we assume that the only objects
// ever handed back from GetObjectByName are tables. Instead we fallthrough
// to the underlying physical accessor.
return l.Accessor.GetObjectDesc(ctx, txn, settings, codec, db, schema, object, flags)
case tree.TableObject:
l.tn = tree.MakeTableNameWithSchema(tree.Name(db), tree.Name(schema), tree.Name(object))
if scEntry, ok := l.vs.GetVirtualSchema(schema); ok {
table, err := scEntry.GetObjectByName(object)
if err != nil {
return nil, err
}
if table == nil {
if flags.Required {
return nil, sqlbase.NewUndefinedRelationError(&l.tn)
}
return nil, nil
}
desc := table.Desc().TableDesc()
if desc == nil {
// This can only happen if we have a non-table object stored on a
// virtual schema. For now we'll return an assertion error.
return nil, errors.AssertionFailedf(
"non-table object of type %T returned from virtual schema for %v",
table.Desc(), l.tn)
}
if flags.RequireMutable {
return sqlbase.NewMutableExistingTableDescriptor(*desc), nil
if scEntry, ok := l.vs.GetVirtualSchema(schema); ok {
desc, err := scEntry.GetObjectByName(object, flags)
if err != nil {
return nil, err
}
if desc == nil {
if flags.Required {
obj := tree.NewQualifiedObjectName(db, schema, object, flags.DesiredObjectKind)
return nil, sqlbase.NewUndefinedObjectError(obj, flags.DesiredObjectKind)
}
return sqlbase.NewImmutableTableDescriptor(*desc), nil
return nil, nil
}

// Fallthrough.
return l.Accessor.GetObjectDesc(ctx, txn, settings, codec, db, schema, object, flags)
default:
return nil, errors.AssertionFailedf("unknown desired object kind %d", flags.DesiredObjectKind)
return desc.Desc(), nil
}
// Fallthrough.
return l.Accessor.GetObjectDesc(ctx, txn, settings, codec, db, schema, object, flags)
}
2 changes: 1 addition & 1 deletion pkg/sql/catalog/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type VirtualSchema interface {
Desc() Descriptor
NumTables() int
VisitTables(func(object VirtualObject))
GetObjectByName(name string) (VirtualObject, error)
GetObjectByName(name string, flags tree.ObjectLookupFlags) (VirtualObject, error)
}

// VirtualObject is a virtual schema object.
Expand Down
4 changes: 1 addition & 3 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -1574,9 +1574,7 @@ func (dsp *DistSQLPlanner) addAggregators(
// the current aggregation e.
argTypes[i] = intermediateTypes[argIdxs[i]]
}
_, outputType, err := execinfrapb.GetAggregateInfo(
finalInfo.Fn, argTypes...,
)
_, outputType, err := execinfrapb.GetAggregateInfo(finalInfo.Fn, argTypes...)
if err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/execinfrapb/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ func (tr *DistSQLTypeResolver) ResolveTypeByID(id uint32) (*types.T, error) {
switch t := typDesc.Kind; t {
case sqlbase.TypeDescriptor_ENUM:
typ = types.MakeEnum(id)
case sqlbase.TypeDescriptor_ALIAS:
return typDesc.Alias, nil
default:
return nil, errors.AssertionFailedf("unknown type kind %s", t)
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/execinfrapb/processors.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,25 @@ func GetAggregateInfo(
}

colTyp := b.FixedReturnType()
// If the output type of the aggregation depends on its inputs, then
// the output of FixedReturnType will be ambiguous. In the ambiguous
// cases, use the information about the input types to construct the
// appropriate output type. The tree.ReturnTyper interface is
// []tree.TypedExpr -> *types.T, so construct the []tree.TypedExpr
// from the types that we know are the inputs. Note that we don't
// try to create datums of each input type, and instead use this
// "TypedDummy" construct. This is because some types don't have resident
// members (like an ENUM with no values), and we shouldn't error out
// trying to pick an aggregate spec for those cases.
if colTyp.IsAmbiguous() {
args := make([]tree.TypedExpr, len(inputTypes))
for i, t := range inputTypes {
args[i] = &tree.TypedDummy{Typ: t}
}
// Evaluate ReturnType with the fake input set of arguments.
colTyp = b.ReturnType(args)
}

return constructAgg, colTyp, nil
}
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/aggregate
Original file line number Diff line number Diff line change
Expand Up @@ -2554,3 +2554,18 @@ SELECT percentile_disc(0.50) FROM osagg

statement error ordered-set aggregations must have a WITHIN GROUP clause containing one ORDER BY column
SELECT percentile_cont(0.50) FROM osagg

# Tests for min/max on collated strings.
statement ok
CREATE TABLE t_collate (x STRING COLLATE en_us);
INSERT INTO t_collate VALUES ('hi' COLLATE en_us), ('hello' COLLATE en_us), ('howdy' COLLATE en_us)

query TT
SELECT min(x), max(x) FROM t_collate
----
hello howdy

query TT
SELECT min(NULL::STRING COLLATE en_us), max(NULL::STRING COLLATE en_us)
----
NULL NULL
Loading

0 comments on commit 126df91

Please sign in to comment.