sql: validate computed columns with

default values while adding

Computed columns with default
values are not validated while adding them, this was causing
issues when data is inserted. Same validation mechanism as insert line
is implemented.

Resolves: cockroachdb#81698
Signed-off-by: Kivanc Bilen <kivanc_bilen@hotmail.com>

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel

@@ -51,6 +51,7 @@ go_library(
         "//pkg/sql/schemachanger/screl",
         "//pkg/sql/sem/catid",
         "//pkg/sql/sem/eval",
+        "//pkg/sql/sem/transform",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sem/volatility",
         "//pkg/sql/sessiondata",

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go

@@ -27,6 +27,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/transform"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
@@ -174,8 +176,31 @@ func alterTableAddColumn(
 		))
 	}
 	if desc.IsComputed() {
+
 		expr := b.ComputedColumnExpression(tbl, d)
 		spec.colType.ComputeExpr = b.WrapExpression(tbl.TableID, expr)
+
+		// run the same validation in sql/row/row_converter.go GenerateInsertRow function
+		var txCtx transform.ExprTransformContext
+		// compute expression can have dependency to another column,
+		// in this case line below will throw an error and validation is not necessary
+		typedExpr, err := tree.TypeCheck(b, expr, b.SemaCtx(), d.Type.(*types.T))
+		if err == nil {
+			typedExpr, err = txCtx.NormalizeExpr(b, b.EvalCtx(), typedExpr)
+			if err != nil {
+				panic(err)
+			}
+			datum, err := eval.Expr(b, b.EvalCtx(), typedExpr)
+			if err != nil {
+				panic(err)
+			}
+			// AdjustValueToType function does the validation
+			_, err = tree.AdjustValueToType(d.Type.(*types.T), datum)
+			if err != nil {
+				panic(err)
+			}
+		}
+
 		if desc.Virtual {
 			b.IncrementSchemaChangeAddColumnQualificationCounter("virtual")
 		} else {
@@ -207,12 +232,14 @@ func alterTableAddColumn(
 	}
 	if desc.HasDefault() {
 		expression := b.WrapExpression(tbl.TableID, cdd.DefaultExpr)
+
 		spec.def = &scpb.ColumnDefaultExpression{
 			TableID:    tbl.TableID,
 			ColumnID:   spec.col.ColumnID,
 			Expression: *expression,
 		}
 		b.IncrementSchemaChangeAddColumnQualificationCounter("default_expr")
+
 	}
 	// We're checking to see if a user is trying add a non-nullable column without a default to a
 	// non-empty table by scanning the primary index span with a limit of 1 to see if any key exists.
@@ -256,6 +283,7 @@ func alterTableAddColumn(
 	default:
 		b.IncrementSchemaChangeAddColumnTypeCounter(spec.colType.Type.TelemetryName())
 	}
+
 }
 
 func columnNamesToIDs(b BuildCtx, tbl *scpb.Table) map[string]descpb.ColumnID {

pkg/sql/tests/BUILD.bazel

@@ -27,6 +27,7 @@ go_test(
     name = "tests_test",
     size = "large",
     srcs = [
+        "add_column_generated_test.go",
         "autocommit_extended_protocol_test.go",
         "bank_test.go",
         "copy_file_upload_test.go",
@@ -91,6 +92,7 @@ go_test(
         "//pkg/sql/sem/builtins",
         "//pkg/sql/sem/builtins/builtinsregistry",
         "//pkg/sql/sem/eval",
+        "//pkg/sql/sem/transform",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sessiondatapb",
         "//pkg/sql/stats",

pkg/sql/tests/add_column_generated_test.go

@@ -0,0 +1,86 @@
+// 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 tests
+
+import (
+	"context"
+	gosql "database/sql"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/stretchr/testify/require"
+)
+
+// TestAddGeneratedColumnValidation tests if validation
+// when creating computed columns is working properly
+func TestAddGeneratedColumnValidation(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+
+	var s serverutils.TestServerInterface
+	params, _ := CreateTestServerParams()
+
+	var db *gosql.DB
+	s, db, _ = serverutils.StartServer(t, params)
+	//sqlDB := sqlutils.MakeSQLRunner(db)
+	defer s.Stopper().Stop(ctx)
+
+	_, err := db.Exec(`
+	CREATE TABLE t (i INT PRIMARY KEY);
+	INSERT INTO t VALUES (1);`)
+	if err != nil {
+		panic(err)
+	}
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN s VARCHAR(2) AS ('st') STORED;`)
+	require.NoError(t, err)
+	//expected := "pq: value too long for type VARCHAR(2)"
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN test INT;`)
+	require.NoError(t, err)
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN ii INT AS (test*2) STORED;`)
+	require.NoError(t, err)
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN iii INT AS (test*3) VIRTUAL;`)
+	require.NoError(t, err)
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN v2 VARCHAR(2) AS ('virtual') VIRTUAL;`)
+	require.Error(t, err)
+	require.True(t, err.Error() == "pq: value too long for type VARCHAR(2)")
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN s2 VARCHAR(2) AS ('stored') STORED;`)
+	require.Error(t, err)
+	require.True(t, err.Error() == "pq: value too long for type VARCHAR(2)")
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN s3 INT2 AS (32768) STORED;`)
+	require.Error(t, err)
+	require.True(t, err.Error() == "pq: integer out of range for type int2")
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN s4 BOOL AS (32768) STORED;`)
+	require.Error(t, err)
+	require.True(t, err.Error() == "pq: expected computed column expression to have type bool, but '32768' has type int")
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN s3 INT2 AS (32768) VIRTUAL;`)
+	require.Error(t, err)
+	require.True(t, err.Error() == "pq: integer out of range for type int2")
+
+	_, err = db.Exec(`ALTER TABLE t ADD COLUMN s4 BOOL AS (32768) VIRTUAL;`)
+	require.Error(t, err)
+	require.True(t, err.Error() == "pq: expected computed column expression to have type bool, but '32768' has type int")
+
+	_, err = db.Exec(`INSERT INTO t VALUES (2);`)
+	require.NoError(t, err)
+
+}