Skip to content

Commit

Permalink
sql: adds unit tests for schemaexpr.CheckConstraintBuilder
Browse files Browse the repository at this point in the history
This commit adds unit tests to the previously untested functions
of `schemaexpra.CheckConstraintBuilder.DefaultName`. It also adds a
`testutils.go` file to the `schemaexpr` package with a function that
makes it easier to create table descriptors that can be used in tests.

Release note: None
  • Loading branch information
mgartner committed Jun 1, 2020
1 parent d78e667 commit aee60f9
Show file tree
Hide file tree
Showing 4 changed files with 251 additions and 36 deletions.
7 changes: 2 additions & 5 deletions pkg/sql/schemaexpr/check_constraint.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ func (b *CheckConstraintBuilder) MarkNameInUse(name string) {
// - It does not include subqueries.
// - It does not include aggregate, window, or set returning functions.
//
// Note that mutable functions are currently allowed, but using them can lead
// to unexpected behavior.
// TODO(mgartner): Add unit tests for Build. Some cases should test
// MarkNameInUse, too.
// Note that mutable functions are currently allowed, unlike in partial index
// predicates, but using them can lead to unexpected behavior.
func (b *CheckConstraintBuilder) Build(
c *tree.CheckConstraintTableDef,
) (*sqlbase.TableDescriptor_CheckConstraint, error) {
Expand Down Expand Up @@ -184,7 +182,6 @@ func (b *CheckConstraintBuilder) generateUniqueName(expr tree.Expr) (string, err
//
// Note that the generated name is not guaranteed to be unique among the other
// constraints of the table.
// TODO(mgartner): Add unit tests for DefaultName.
func (b *CheckConstraintBuilder) DefaultName(expr tree.Expr) (string, error) {
var nameBuf bytes.Buffer
nameBuf.WriteString("check")
Expand Down
193 changes: 193 additions & 0 deletions pkg/sql/schemaexpr/check_constraint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package schemaexpr

import (
"context"
"testing"

"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/builtins"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
)

func TestCheckConstraintBuilder_MarkNameInUse(t *testing.T) {
// TODO(mgartner);
}

func TestCheckConstraintBuilder_Build(t *testing.T) {
ctx := context.Background()
semaCtx := tree.MakeSemaContext()

// Trick to get the init() for the builtins package to run.
_ = builtins.AllBuiltinNames

database := tree.Name("foo")
table := tree.Name("bar")
tn := tree.MakeTableName(database, table)

desc := testTableDesc(
string(table),
[]testCol{{"a", types.Bool}, {"b", types.Int}},
[]testCol{{"c", types.String}},
)

builder := NewCheckConstraintBuilder(ctx, tn, &desc, &semaCtx)
builder.MarkNameInUse("check_a3")

testData := []struct {
name string
expr string
expectedValid bool
expectedExpr string
expectedName string
}{
// Respect custom names.
{"chk_1", "a", true, "a", "chk_1"},

// Use unique default names when there is no custom name.
{"", "a", true, "a", "check_a"},
{"", "a", true, "a", "check_a1"},
{"", "a", true, "a", "check_a2"},
{"", "a AND b = 0", true, "a AND (b = 0)", "check_a_b"},
{"", "a AND b = 1", true, "a AND (b = 1)", "check_a_b1"},
{"", "a AND b = 1", true, "a AND (b = 1)", "check_a_b2"},

// Respect that "check_a3" has been marked, so the next check constraint
// with "a" should be "check_a4".
{"", "a", true, "a", "check_a4"},
{"", "a", true, "a", "check_a5"},

// Allow expressions that result in a bool.
{"ck", "a", true, "a", "ck"},
{"ck", "b = 0", true, "b = 0", "ck"},
{"ck", "a AND b = 0", true, "a AND (b = 0)", "ck"},
{"ck", "a IS NULL", true, "a IS NULL", "ck"},
{"ck", "b IN (1, 2)", true, "b IN (1:::INT8, 2:::INT8)", "ck"},

// Allow immutable functions.
{"ck", "abs(b) > 0", true, "abs(b) > 0", "ck"},
{"ck", "c || c = 'foofoo'", true, "(c || c) = 'foofoo'", "ck"},
{"ck", "lower(c) = 'bar'", true, "lower(c) = 'bar'", "ck"},

// Allow mutable functions.
{"ck", "b > random()", true, "b > random()", "ck"},

// Disallow references to columns not in the table.
{"", "d", false, "", ""},
{"", "t.a", false, "", ""},

// Disallow expressions that do not result in a bool.
{"", "b", false, "", ""},
{"", "abs(b)", false, "", ""},
{"", "lower(c)", false, "", ""},

// Disallow subqueries.
{"", "exists(select 1)", false, "", ""},
{"", "b IN (select 1)", false, "", ""},

// Disallow aggregate, window, and set returning functions.
{"", "sum(b) > 10", false, "", ""},
{"", "row_number() OVER () > 1", false, "", ""},
{"", "generate_series(1, 1) > 2", false, "", ""},

// Dequalify column names.
{"ck", "bar.a", true, "a", "ck"},
{"ck", "foo.bar.a", true, "a", "ck"},
{"ck", "bar.b = 0", true, "b = 0", "ck"},
{"ck", "foo.bar.b = 0", true, "b = 0", "ck"},
{"ck", "bar.a AND foo.bar.b = 0", true, "a AND (b = 0)", "ck"},
}

for _, d := range testData {
t.Run(d.expr, func(t *testing.T) {
expr, err := parser.ParseExpr(d.expr)
if err != nil {
t.Fatalf("%s: unexpected error: %s", d.expr, err)
}

ckDef := &tree.CheckConstraintTableDef{Name: tree.Name(d.name), Expr: expr}

ck, err := builder.Build(ckDef)

if !d.expectedValid {
if err == nil {
t.Fatalf("%s: expected invalid expression, but was valid", d.expr)
}
// The input expression is invalid so there is no need to check
// the output ck.
return
}

if err != nil {
t.Fatalf("%s: expected valid expression, but found error: %s", d.expr, err)
}

if ck.Name != d.expectedName || ck.Expr != d.expectedExpr {
t.Errorf(
`%s: expected "%s CHECK %s", got "%s CHECK %s"`,
d.expr,
d.expectedName,
d.expectedExpr,
ck.Name,
ck.Expr,
)
}
})
}
}

func TestCheckConstraintBuilder_DefaultName(t *testing.T) {
ctx := context.Background()
semaCtx := tree.MakeSemaContext()

database := tree.Name("foo")
table := tree.Name("bar")
tn := tree.MakeTableName(database, table)

desc := testTableDesc(
string(table),
[]testCol{{"a", types.Bool}, {"b", types.Int}},
[]testCol{{"c", types.String}, {"d", types.String}, {"e", types.String}},
)

builder := NewCheckConstraintBuilder(ctx, tn, &desc, &semaCtx)

testData := []struct {
expr string
expected string
}{
{"a > 0", "check_a"},
{"a > 0 AND b = 'foo'", "check_a_b"},
{"a > 0 AND (b = 'foo' OR a < 20)", "check_a_b_a"},
{"a > 0 AND a < 20", "check_a_a"},
{"((a AND (b OR c)) AND d) OR e", "check_a_b_c_d_e"},
}

for _, d := range testData {
t.Run(d.expr, func(t *testing.T) {
expr, err := parser.ParseExpr(d.expr)
if err != nil {
t.Fatalf("%s: unexpected error: %s", d.expr, err)
}

r, err := builder.DefaultName(expr)
if err != nil {
t.Fatalf("%s: expected success, but found error: %s", d.expr, err)
}

if r != d.expected {
t.Errorf("%s: expected %q, got %q", d.expr, d.expected, r)
}
})
}
}
36 changes: 5 additions & 31 deletions pkg/sql/schemaexpr/partial_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/builtins"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/sql/types"
)

Expand All @@ -32,36 +31,11 @@ func TestIndexPredicateValidator_Validate(t *testing.T) {
table := tree.Name("bar")
tn := tree.MakeTableName(database, table)

desc := sqlbase.MutableTableDescriptor{
TableDescriptor: sqlbase.TableDescriptor{
Name: string(table),
ID: 1,
Columns: []sqlbase.ColumnDescriptor{
{
Name: "a",
ID: 1,
Type: types.Bool,
},
{
Name: "b",
ID: 2,
Type: types.Int,
},
},
Mutations: []sqlbase.DescriptorMutation{
{
Descriptor_: &sqlbase.DescriptorMutation_Column{
Column: &sqlbase.ColumnDescriptor{
Name: "c",
ID: 3,
Type: types.String,
},
},
Direction: sqlbase.DescriptorMutation_ADD,
},
},
},
}
desc := testTableDesc(
string(table),
[]testCol{{"a", types.Bool}, {"b", types.Int}},
[]testCol{{"c", types.String}},
)

validator := NewIndexPredicateValidator(ctx, tn, &desc, &semaCtx)

Expand Down
51 changes: 51 additions & 0 deletions pkg/sql/schemaexpr/testutils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package schemaexpr

import (
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/sql/types"
)

// testCol includes the information needed to create a column descriptor for
// testing purposes.
type testCol struct {
name string
typ *types.T
}

// testTableDesc is a helper functions for creating table descriptors in a
// less verbose way.
func testTableDesc(
name string, columns []testCol, mutationColumns []testCol,
) sqlbase.MutableTableDescriptor {
cols := make([]sqlbase.ColumnDescriptor, len(columns))
for i := range columns {
cols[i] = sqlbase.ColumnDescriptor{
Name: columns[i].name,
Type: columns[i].typ,
ID: sqlbase.ColumnID(i),
}
}

muts := make([]sqlbase.DescriptorMutation, len(mutationColumns))
for i := range mutationColumns {
muts[i] = sqlbase.DescriptorMutation{
Descriptor_: &sqlbase.DescriptorMutation_Column{
Column: &sqlbase.ColumnDescriptor{
Name: mutationColumns[i].name,
Type: mutationColumns[i].typ,
ID: sqlbase.ColumnID(len(columns) + i),
},
},
Direction: sqlbase.DescriptorMutation_ADD,
}
}

return sqlbase.MutableTableDescriptor{
TableDescriptor: sqlbase.TableDescriptor{
Name: name,
ID: 1,
Columns: cols,
Mutations: muts,
},
}
}

0 comments on commit aee60f9

Please sign in to comment.