Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
70469: builtins: use IdentityReturnType when possible r=otan a=rafiss

fixes #70475

see individual commits

Release note: None

70572: ui: optimize cluster-ui build r=jordanlewis,Azhng a=koorosh

Before, `cluster-ui` webpack configuration preprocessed
protobuf's generated code with babel and it took significant
amount of time. With current change, webpack configuration is
updated to exclude protobufs bundle from preprocessing and
now it is bundled "as is" into main bundle.

Release note: none

70598: vendor: bump Pebble to 9656de4a3019 r=jbowens a=jbowens

```
9656de4 db: write OPTIONS file to temporary path
558d587 internal/cache: prevent coldTarget from becoming greater than targetSize
634eb34 db: support nil logger in EventListener.EnsureDefaults
d2266ba internal/cache: update entry size in the etTest to etHot transition
```

Bumping now because I need 634eb34 for fixing the disk-full roachtest.

Release note: None

70606: builtins: array builtins handle mismatched tuple types r=otan a=rafiss

fixes #70481
fixes #70480
fixes #70479
fixes #70478
fixes #70477
fixes #70476

The first commit is a refactor to add a new CompareError method
to Datum. It propagates errors instead of panicking.

Since these builtins take in AnyTuple, the tuple contents aren't
type-checked. Instead, we must handle any errors that occur during tuple
comparison at execution-time. This matches the PostgresSQL behavior.

No release note since this bug was never released anywhere.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Andrii Vorobiov <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
  • Loading branch information
4 people committed Sep 23, 2021
5 parents 8438882 + 23f4ee3 + 3a70acd + 689221c + dda6518 commit 55ef033
Show file tree
Hide file tree
Showing 12 changed files with 516 additions and 138 deletions.
4 changes: 2 additions & 2 deletions DEPS.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -651,8 +651,8 @@ def go_deps():
name = "com_github_cockroachdb_pebble",
build_file_proto_mode = "disable_global",
importpath = "github.com/cockroachdb/pebble",
sum = "h1:BV8fDvAogQeaAUCgx/8/6J/Sv/enyfkJ10l5bTAcQWI=",
version = "v0.0.0-20210921140715-9509dcb7a53a",
sum = "h1:1eWG3mTDzXwh7WFCZt8pO3bw/xnGLJ9kAHVmoqxDSWE=",
version = "v0.0.0-20210922174139-9656de4a3019",
)

go_repository(
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ require (
github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
github.com/cockroachdb/gostdlib v1.13.0
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f
github.com/cockroachdb/pebble v0.0.0-20210921140715-9509dcb7a53a
github.com/cockroachdb/pebble v0.0.0-20210922174139-9656de4a3019
github.com/cockroachdb/redact v1.1.3
github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ github.com/cockroachdb/gostdlib v1.13.0 h1:TzSEPYgkKDNei3gbLc0rrHu4iHyBp7/+NxPOF
github.com/cockroachdb/gostdlib v1.13.0/go.mod h1:eXX95p9QDrYwJfJ6AgeN9QnRa/lqqid9LAzWz/l5OgA=
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f h1:o/kfcElHqOiXqcou5a3rIlMc7oJbMQkeLk0VQJ7zgqY=
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
github.com/cockroachdb/pebble v0.0.0-20210921140715-9509dcb7a53a h1:BV8fDvAogQeaAUCgx/8/6J/Sv/enyfkJ10l5bTAcQWI=
github.com/cockroachdb/pebble v0.0.0-20210921140715-9509dcb7a53a/go.mod h1:JXfQr3d+XO4bL1pxGwKKo09xylQSdZ/mpZ9b2wfVcPs=
github.com/cockroachdb/pebble v0.0.0-20210922174139-9656de4a3019 h1:1eWG3mTDzXwh7WFCZt8pO3bw/xnGLJ9kAHVmoqxDSWE=
github.com/cockroachdb/pebble v0.0.0-20210922174139-9656de4a3019/go.mod h1:JXfQr3d+XO4bL1pxGwKKo09xylQSdZ/mpZ9b2wfVcPs=
github.com/cockroachdb/redact v1.0.8/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
github.com/cockroachdb/redact v1.1.0/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
github.com/cockroachdb/redact v1.1.3 h1:AKZds10rFSIj7qADf0g46UixK8NNLwWTNdCIGS5wfSQ=
Expand Down
39 changes: 39 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/array
Original file line number Diff line number Diff line change
Expand Up @@ -1938,3 +1938,42 @@ query T
SELECT array_positions(NULL::record[], ROW(33,'hippo'))
----
NULL

# Handle tuples with mismatched types. This behavior matches PostgreSQL -- the
# tuple comparison will short-circuit, so for example, there will be no error
# if the second elements' types don't match as long as the first elements are
# unequal.

query error pgcode 42804 unsupported comparison.*\n.*at record column 2
SELECT array_replace(ARRAY[(1,'cat'),(2,'dog')], (1, 12.3::decimal), (3,'fish'))

# array_append does not require the tuple types to match exactly.
query error pgcode 42804 unsupported comparison.*\n.*at record column 2
SELECT array_replace(array_append(array[(1,'cat')], (2,true)), (2,'fish'), (3,'dog'))

# This works because the comparison short-circuits before comparing the boolean column
query T
SELECT array_replace(array_append(array[(1,'cat')], (2,true)), (1,'fish'), (3,'dog'))
----
{"(1,cat)","(2,t)"}

# The value that replaces the old one can have types.
query T
SELECT array_replace(ARRAY[(1,'cat'),(2,'dog')], (2, 'dog'), (3,true))
----
{"(1,cat)","(3,t)"}

query error pgcode 42804 unsupported comparison.*\n.*at record column 1
SELECT array_position(ARRAY[(12::OID,NULL),(NULL,NULL)], ('\xc03b4478eb'::BYTEA,NULL))

query error pgcode 42804 unsupported comparison.*\n.*at record column 2
SELECT array_positions(ARRAY[(1,'cat'),(2,'dog')], (2,true))

query error pgcode 42804 unsupported comparison.*\n.*at record column 2
SELECT array_remove(ARRAY[(1,'cat'),(2,'dog')], (2,'\xc03b4478eb'::BYTEA))

# This works because the comparison short-circuits before comparing the BYTEA column
query T
SELECT array_remove(ARRAY[(1,'cat'),(2,'dog')], (3,'\xc03b4478eb'::BYTEA))
----
{"(1,cat)","(2,dog)"}
19 changes: 17 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/json
Original file line number Diff line number Diff line change
Expand Up @@ -899,12 +899,27 @@ d
query error could not parse \"foo\" as type int
SELECT json_populate_record(((3,) AS a), '{"a": "foo"}')

query error anonymous records cannot be used with json_populate_record
query error anonymous records cannot be used with json{b}_populate_record{set}
SELECT * FROM json_populate_record((1,2,3,4), '{"a": 3, "c": 10, "d": 11.001}')

query error anonymous records cannot be used with json_populate_record
query error anonymous records cannot be used with json{b}_populate_record{set}
SELECT * FROM json_populate_record(NULL, '{"a": 3, "c": 10, "d": 11.001}')

query error first argument of json{b}_populate_record{set} must be a record type
SELECT * FROM json_populate_record(1, '{"a": 3, "c": 10, "d": 11.001}')

query error first argument of json{b}_populate_record{set} must be a record type
SELECT * FROM json_populate_record(NULL::INT, '{"a": 3, "c": 10, "d": 11.001}')

query error anonymous records cannot be used with json{b}_populate_record{set}
SELECT * FROM json_populate_record(NULL::record, '{"a": 3, "c": 10, "d": 11.001}')

query error anonymous records cannot be used with json{b}_populate_record{set}
SELECT * FROM json_populate_recordset(NULL, '[{"a": 3, "c": 10, "d": 11.001}, {}]')

query error first argument of json{b}_populate_record{set} must be a record type
SELECT * FROM json_populate_recordset(NULL::INT, '[{"a": 3, "c": 10, "d": 11.001}, {}]')

query I
SELECT * FROM json_populate_record(((3,) AS a), NULL)
----
Expand Down
32 changes: 24 additions & 8 deletions pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -3434,7 +3434,7 @@ value if you rely on the HLC for accuracy.`,
"array_append": setProps(arrayPropsNullableArgs(), arrayBuiltin(func(typ *types.T) tree.Overload {
return tree.Overload{
Types: tree.ArgTypes{{"array", types.MakeArray(typ)}, {"elem", typ}},
ReturnType: tree.FixedReturnType(types.MakeArray(typ)),
ReturnType: tree.IdentityReturnType(0),
Fn: func(_ *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
return tree.AppendToMaybeNullArray(typ, args[0], args[1])
},
Expand All @@ -3446,7 +3446,7 @@ value if you rely on the HLC for accuracy.`,
"array_prepend": setProps(arrayPropsNullableArgs(), arrayBuiltin(func(typ *types.T) tree.Overload {
return tree.Overload{
Types: tree.ArgTypes{{"elem", typ}, {"array", types.MakeArray(typ)}},
ReturnType: tree.FixedReturnType(types.MakeArray(typ)),
ReturnType: tree.IdentityReturnType(1),
Fn: func(_ *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
return tree.PrependToMaybeNullArray(typ, args[0], args[1])
},
Expand All @@ -3470,14 +3470,18 @@ value if you rely on the HLC for accuracy.`,
"array_remove": setProps(arrayPropsNullableArgs(), arrayBuiltin(func(typ *types.T) tree.Overload {
return tree.Overload{
Types: tree.ArgTypes{{"array", types.MakeArray(typ)}, {"elem", typ}},
ReturnType: tree.FixedReturnType(types.MakeArray(typ)),
ReturnType: tree.IdentityReturnType(0),
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
if args[0] == tree.DNull {
return tree.DNull, nil
}
result := tree.NewDArray(typ)
for _, e := range tree.MustBeDArray(args[0]).Array {
if e.Compare(ctx, args[1]) != 0 {
cmp, err := e.CompareError(ctx, args[1])
if err != nil {
return nil, err
}
if cmp != 0 {
if err := result.Append(e); err != nil {
return nil, err
}
Expand All @@ -3493,14 +3497,18 @@ value if you rely on the HLC for accuracy.`,
"array_replace": setProps(arrayPropsNullableArgs(), arrayBuiltin(func(typ *types.T) tree.Overload {
return tree.Overload{
Types: tree.ArgTypes{{"array", types.MakeArray(typ)}, {"toreplace", typ}, {"replacewith", typ}},
ReturnType: tree.FixedReturnType(types.MakeArray(typ)),
ReturnType: tree.IdentityReturnType(0),
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
if args[0] == tree.DNull {
return tree.DNull, nil
}
result := tree.NewDArray(typ)
for _, e := range tree.MustBeDArray(args[0]).Array {
if e.Compare(ctx, args[1]) == 0 {
cmp, err := e.CompareError(ctx, args[1])
if err != nil {
return nil, err
}
if cmp == 0 {
if err := result.Append(args[2]); err != nil {
return nil, err
}
Expand All @@ -3526,7 +3534,11 @@ value if you rely on the HLC for accuracy.`,
return tree.DNull, nil
}
for i, e := range tree.MustBeDArray(args[0]).Array {
if e.Compare(ctx, args[1]) == 0 {
cmp, err := e.CompareError(ctx, args[1])
if err != nil {
return nil, err
}
if cmp == 0 {
return tree.NewDInt(tree.DInt(i + 1)), nil
}
}
Expand All @@ -3547,7 +3559,11 @@ value if you rely on the HLC for accuracy.`,
}
result := tree.NewDArray(types.Int)
for i, e := range tree.MustBeDArray(args[0]).Array {
if e.Compare(ctx, args[1]) == 0 {
cmp, err := e.CompareError(ctx, args[1])
if err != nil {
return nil, err
}
if cmp == 0 {
if err := result.Append(tree.NewDInt(tree.DInt(i + 1))); err != nil {
return nil, err
}
Expand Down
19 changes: 11 additions & 8 deletions pkg/sql/sem/builtins/generator_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,9 @@ func makeJSONPopulateImpl(gen tree.GeneratorWithExprsFactory, info string) tree.
// The json{,b}_populate_record{,set} builtins all have a 2 argument
// structure. The first argument is an arbitrary tuple type, which is used
// to set the columns of the output when the builtin is used as a FROM
// source, or used as-is when it's used as an ordinary projection.
// source, or used as-is when it's used as an ordinary projection. To match
// PostgreSQL, the argument actually is types.Any, and its tuple-ness is
// checked at execution time.
// The second argument is a JSON object or array of objects. The builtin
// transforms the JSON in the second argument into the tuple in the first
// argument, field by field, casting fields in key "k" to the type in the
Expand All @@ -1261,13 +1263,8 @@ func makeJSONPopulateImpl(gen tree.GeneratorWithExprsFactory, info string) tree.
// the default values of each field will be NULL.
// The second argument can also be null, in which case the first argument
// is returned as-is.
Types: tree.ArgTypes{{"base", types.Any}, {"from_json", types.Jsonb}},
ReturnType: func(args []tree.TypedExpr) *types.T {
if len(args) != 2 {
return tree.UnknownReturnType
}
return args[0].ResolvedType()
},
Types: tree.ArgTypes{{"base", types.Any}, {"from_json", types.Jsonb}},
ReturnType: tree.IdentityReturnType(0),
GeneratorWithExprs: gen,
Info: info,
Volatility: tree.VolatilityStable,
Expand Down Expand Up @@ -1311,6 +1308,12 @@ func jsonPopulateRecordEvalArgs(
}
}
tupleType := args[0].(tree.TypedExpr).ResolvedType()
if tupleType.Family() != types.TupleFamily && tupleType.Family() != types.UnknownFamily {
return nil, nil, pgerror.New(
pgcode.DatatypeMismatch,
"first argument of json{b}_populate_record{set} must be a record type",
)
}
var defaultElems tree.Datums
if evalled[0] == tree.DNull {
defaultElems = make(tree.Datums, len(tupleType.TupleLabels()))
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/sem/tree/casts.go
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,10 @@ func PopulateRecordWithJSON(
tupleTypes := desiredType.TupleContents()
labels := desiredType.TupleLabels()
if labels == nil {
return pgerror.Newf(pgcode.InvalidParameterValue, "anonymous records cannot be used with json_populate_record")
return pgerror.Newf(
pgcode.InvalidParameterValue,
"anonymous records cannot be used with json{b}_populate_record{set}",
)
}
for i := range tupleTypes {
val, err := j.FetchValKey(labels[i])
Expand Down
Loading

0 comments on commit 55ef033

Please sign in to comment.