Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: Allow DEFAULT/ON UPDATE types that can be assignment-cast #81071

Merged
merged 2 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/avro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func parseValues(tableDesc catalog.TableDescriptor, values string) ([]rowenc.Enc
for colIdx, expr := range rowTuple {
col := tableDesc.PublicColumns()[colIdx]
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(
ctx, expr, col.GetType(), "avro", &semaCtx, volatility.Stable)
ctx, expr, col.GetType(), "avro", &semaCtx, volatility.Stable, false /*allowAssignmentCast*/)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/cdceval/expr_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func (e *exprEval) typeCheck(
) (tree.TypedExpr, error) {
// If we have variable free immutable expressions, then we can just evaluate it right away.
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(
ctx, expr, targetType, "cdc", &e.semaCtx, volatility.Immutable)
ctx, expr, targetType, "cdc", &e.semaCtx, volatility.Immutable, false)
if err == nil {
d, err := eval.Expr(e.evalCtx, typedExpr)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/partitionccl/partition.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ func valueEncodePartitionTuple(
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(evalCtx.Context, expr, cols[i].GetType(), "partition",
&semaCtx,
volatility.Immutable,
false, /*allowAssignmentCast*/
)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,7 @@ func sanitizeColumnExpression(
) (tree.TypedExpr, string, error) {
colDatumType := col.GetType()
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(
p.ctx, expr, colDatumType, opName, &p.p.semaCtx, volatility.Volatile,
p.ctx, expr, colDatumType, opName, &p.p.semaCtx, volatility.Volatile, false, /*allowAssignmentCast*/
)
if err != nil {
return nil, "", pgerror.WithCandidateCode(err, pgcode.DatatypeMismatch)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/alter_table_locality.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ func (n *alterTableSetLocalityNode) alterTableLocalityToRegionalByRow(
"REGIONAL BY ROW DEFAULT",
params.p.SemaCtx(),
volatility.Volatile,
false, /*allowAssignmentCast*/
)
if err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/analyze_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ func (p *planner) analyzeExpr(
var err error
p.semaCtx.IVarContainer = iVarHelper.Container()
if requireType {
typedExpr, err = tree.TypeCheckAndRequire(ctx, resolved, &p.semaCtx,
expectedType, typingContext)
typedExpr, err = tree.TypeCheckAndRequire(ctx, resolved, &p.semaCtx, expectedType, typingContext)
} else {
typedExpr, err = tree.TypeCheck(ctx, resolved, &p.semaCtx, expectedType)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/schemaexpr/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ go_library(
"//pkg/sql/parser",
"//pkg/sql/pgwire/pgcode",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/sem/cast",
"//pkg/sql/sem/catid",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/transform",
Expand Down
12 changes: 10 additions & 2 deletions pkg/sql/catalog/schemaexpr/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/cast"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sem/volatility"
Expand Down Expand Up @@ -105,6 +106,7 @@ func DequalifyAndValidateExpr(
context,
semaCtx,
maxVolatility,
false, /*allowAssignmentCast*/
)

if err != nil {
Expand Down Expand Up @@ -295,7 +297,7 @@ func deserializeExprForFormatting(
// typedExpr.
if fmtFlags == tree.FmtPGCatalog {
sanitizedExpr, err := SanitizeVarFreeExpr(ctx, expr, typedExpr.ResolvedType(), "FORMAT", semaCtx,
volatility.Immutable)
volatility.Immutable, false /*allowAssignmentCast*/)
// If the expr has no variables and has Immutable, we can evaluate
// it and turn it into a constant.
if err == nil {
Expand Down Expand Up @@ -398,6 +400,7 @@ func SanitizeVarFreeExpr(
context string,
semaCtx *tree.SemaContext,
maxVolatility volatility.V,
allowAssignmentCast bool,
) (tree.TypedExpr, error) {
if tree.ContainsVars(expr) {
return nil, pgerror.Newf(pgcode.Syntax,
Expand Down Expand Up @@ -436,7 +439,12 @@ func SanitizeVarFreeExpr(
actualType := typedExpr.ResolvedType()
if !expectedType.Equivalent(actualType) && typedExpr != tree.DNull {
// The expression must match the column type exactly unless it is a constant
// NULL value.
// NULL value or assignment casts are allowed.
if allowAssignmentCast {
if ok := cast.ValidCast(actualType, expectedType, cast.ContextAssignment); ok {
return typedExpr, nil
}
}
return nil, fmt.Errorf("expected %s expression to have type %s, but '%s' has type %s",
context, expectedType, expr, actualType)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/catalog/tabledesc/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func MakeColumnDefDescs(
// Verify the default expression type is compatible with the column type
// and does not contain invalid functions.
ret.DefaultExpr, err = schemaexpr.SanitizeVarFreeExpr(
ctx, d.DefaultExpr.Expr, resType, "DEFAULT", semaCtx, volatility.Volatile,
ctx, d.DefaultExpr.Expr, resType, "DEFAULT", semaCtx, volatility.Volatile, true, /*allowAssignmentCast*/
)
if err != nil {
return nil, err
Expand All @@ -181,7 +181,7 @@ func MakeColumnDefDescs(
// Verify the on update expression type is compatible with the column type
// and does not contain invalid functions.
ret.OnUpdateExpr, err = schemaexpr.SanitizeVarFreeExpr(
ctx, d.OnUpdateExpr.Expr, resType, "ON UPDATE", semaCtx, volatility.Volatile,
ctx, d.OnUpdateExpr.Expr, resType, "ON UPDATE", semaCtx, volatility.Volatile, true, /*allowAssignmentCast*/
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -266,7 +266,7 @@ func EvalShardBucketCount(
shardBuckets = paramVal
}
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(
ctx, shardBuckets, types.Int, "BUCKET_COUNT", semaCtx, volatility.Volatile,
ctx, shardBuckets, types.Int, "BUCKET_COUNT", semaCtx, volatility.Volatile, false, /*allowAssignmentCast*/
)
if err != nil {
return 0, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (p *planner) fillInPlaceholders(
}
}
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(
ctx, e, typ, "EXECUTE parameter" /* context */, &semaCtx, volatility.Volatile,
ctx, e, typ, "EXECUTE parameter" /* context */, &semaCtx, volatility.Volatile, false, /*allowAssignmentCast*/
)
if err != nil {
return nil, pgerror.WithCandidateCode(err, pgcode.WrongObjectType)
Expand Down
68 changes: 68 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/cast
Original file line number Diff line number Diff line change
Expand Up @@ -1418,3 +1418,71 @@ query TT
SELECT (-1/3.0)::float4::text, (1/3.0)::float8::text
----
-0.3 0.3

# Test that default/on update expression is allowed to differ from column type
e-mbrown marked this conversation as resolved.
Show resolved Hide resolved
statement ok
CREATE TABLE def_assn_cast (
id INT4,
a INT4 DEFAULT 1.0::FLOAT4,
b VARCHAR DEFAULT 'true'::BOOL,
c NAME DEFAULT 'foo'::BPCHAR
)

statement ok
CREATE TABLE upd_assn_cast (
id INT4,
a TEXT ON UPDATE 'POINT(2.0 2.0)'::GEOMETRY,
b FLOAT8 ON UPDATE 6::INT4,
c VARCHAR ON UPDATE '{ "customer": "John Doe"}'::JSONB
)

# Ensure insertions are allowed
statement ok
INSERT INTO def_assn_cast(id) VALUES (1)

statement ok
INSERT INTO upd_assn_cast(id) VALUES (1)

statement ok
UPDATE upd_assn_cast SET id = 2

query IITT
SELECT * from def_assn_cast
----
1 1 true f

query ITFT
SELECT * from upd_assn_cast
----
2 010100000000000000000000400000000000000040 6 {"customer": "John Doe"}


statement error pq: could not parse .* as type .*: invalid .* value
CREATE TABLE fail_assn_cast (
a BOOL DEFAULT 'foo'
)

statement error pq: expected DEFAULT expression to have type .*, but .* has type .*
CREATE TABLE fail_assn_cast (
a DATE DEFAULT 1.0::FLOAT4
)

statement error pq: expected DEFAULT expression to have type .*, but .* has type .*
CREATE TABLE fail_assn_cast (
b JSONB DEFAULT 'null'::CHAR
)

statement error pq: expected ON UPDATE expression to have type .*, but .* has type .*
CREATE TABLE fail_assn_cast (
a INT4 ON UPDATE 1.0::BOOL
)

statement error pq: expected ON UPDATE expression to have type .*, but .* has type .*
CREATE TABLE fail_assn_cast (
a FLOAT4 ON UPDATE '01/02/03'::DATE
)

statement error pq: expected ON UPDATE expression to have type .*, but .* has type .*
CREATE TABLE fail_assn_cast (
a NUMERIC ON UPDATE '1-2'::INTERVAL
)
1 change: 1 addition & 0 deletions pkg/sql/opt/bench/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ func newHarness(tb testing.TB, query benchQuery) *harness {
"", /* context */
&h.semaCtx,
volatility.Volatile,
false, /*allowAssignmentCast*/
)
if err != nil {
tb.Fatalf("%v", err)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/opt/testutils/opttester/opt_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,7 @@ func (ot *OptTester) AssignPlaceholders(
"", /* context */
&ot.semaCtx,
volatility.Volatile,
false, /*allowAssignmentCast*/
)
if err != nil {
return nil, err
Expand Down
4 changes: 1 addition & 3 deletions pkg/sql/opt/testutils/testcat/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ func injectTableStats(tt *Table, statsExpr tree.Expr) {
ctx := context.Background()
semaCtx := tree.MakeSemaContext()
evalCtx := eval.MakeTestingEvalContext(cluster.MakeTestingClusterSettings())
typedExpr, err := tree.TypeCheckAndRequire(
ctx, statsExpr, &semaCtx, types.Jsonb, "INJECT STATISTICS",
)
typedExpr, err := tree.TypeCheckAndRequire(ctx, statsExpr, &semaCtx, types.Jsonb, "INJECT STATISTICS")
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/row/expr_walker.go
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ func sanitizeExprsForImport(

// If we have immutable expressions, then we can just return it right away.
typedExpr, err := schemaexpr.SanitizeVarFreeExpr(
ctx, expr, targetType, "import_default", &semaCtx, volatility.Immutable)
ctx, expr, targetType, "import_default", &semaCtx, volatility.Immutable, false /*allowAssignmentCast*/)
if err == nil {
return typedExpr, overrideImmutable, nil
}
Expand Down