fixed function call in default value of a column and added tests

Signed-off-by: GuptaManan100 <manan@planetscale.com>

.github/workflows/cluster_endtoend_mysql80.yml

@@ -0,0 +1,50 @@
+# DO NOT MODIFY: THIS FILE IS GENERATED USING "make generate_ci_workflows"
+
+name: Cluster (mysql80)
+on: [push, pull_request]
+jobs:
+
+  build:
+    name: Run endtoend tests on Cluster (mysql80)
+    runs-on: ubuntu-20.04
+
+    steps:
+    - name: Set up Go
+      uses: actions/setup-go@v1
+      with:
+        go-version: 1.16
+
+    - name: Tune the OS
+      run: |
+        echo '1024 65535' | sudo tee -a /proc/sys/net/ipv4/ip_local_port_range
+
+    # TEMPORARY WHILE GITHUB FIXES THIS https://github.com/actions/virtual-environments/issues/3185
+    - name: Add the current IP address, long hostname and short hostname record to /etc/hosts file
+      run: |
+        echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
+    # DON'T FORGET TO REMOVE CODE ABOVE WHEN ISSUE IS ADRESSED!
+
+    - name: Check out code
+      uses: actions/checkout@v2
+
+    - name: Get dependencies
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y mysql-server mysql-client make unzip g++ etcd curl git wget eatmydata
+        sudo service mysql stop
+        sudo service etcd stop
+        sudo ln -s /etc/apparmor.d/usr.sbin.mysqld /etc/apparmor.d/disable/
+        sudo apparmor_parser -R /etc/apparmor.d/usr.sbin.mysqld
+        go mod download
+
+        wget https://repo.percona.com/apt/percona-release_latest.$(lsb_release -sc)_all.deb
+        sudo apt-get install -y gnupg2
+        sudo dpkg -i percona-release_latest.$(lsb_release -sc)_all.deb
+        sudo apt-get update
+        sudo apt-get install percona-xtrabackup-24
+
+    - name: Run cluster endtoend test
+      timeout-minutes: 30
+      run: |
+        source build.env
+        eatmydata -- go run test.go -docker=false -print-log -follow -shard mysql80

go/test/endtoend/vtgate/main_test.go

@@ -434,6 +434,7 @@ func TestMain(m *testing.M) {
 			return 1
 		}
 
+		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "-enable_system_settings=true")
 		// Start vtgate
 		err = clusterInstance.StartVtgate()
 		if err != nil {

go/test/endtoend/vtgate/misc_test.go

@@ -659,6 +659,51 @@ func TestDeleteAlias(t *testing.T) {
 	exec(t, conn, "delete t.* from t1 t where t.id1 = 1")
 }
 
+func TestFunctionInDefault(t *testing.T) {
+	defer cluster.PanicHandler(t)
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	// set the sql mode ALLOW_INVALID_DATES
+	exec(t, conn, `SET sql_mode = 'ALLOW_INVALID_DATES'`)
+
+	_, err = conn.ExecuteFetch(`create table function_default (x varchar(25) DEFAULT (TRIM(" check ")))`, 1000, true)
+	// this query fails because mysql57 does not support functions in default clause
+	require.Error(t, err)
+
+	// verify that currenet_timestamp and it's aliases work as default values
+	exec(t, conn, `create table function_default (
+ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+dt DATETIME DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ts2 TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
+dt2 DATETIME DEFAULT CURRENT_TIMESTAMP,
+ts3 TIMESTAMP DEFAULT 0,
+dt3 DATETIME DEFAULT 0,
+ts4 TIMESTAMP DEFAULT 0 ON UPDATE CURRENT_TIMESTAMP,
+dt4 DATETIME DEFAULT 0 ON UPDATE CURRENT_TIMESTAMP,
+ts5 TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ts6 TIMESTAMP NULL ON UPDATE CURRENT_TIMESTAMP,
+dt5 DATETIME ON UPDATE CURRENT_TIMESTAMP,
+dt6 DATETIME NOT NULL ON UPDATE CURRENT_TIMESTAMP,
+ts7 TIMESTAMP(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),
+ts8 TIMESTAMP DEFAULT NOW(),
+ts9 TIMESTAMP DEFAULT LOCALTIMESTAMP,
+ts10 TIMESTAMP DEFAULT LOCALTIME,
+ts11 TIMESTAMP DEFAULT LOCALTIMESTAMP(),
+ts12 TIMESTAMP DEFAULT LOCALTIME()
+)`)
+	exec(t, conn, "drop table function_default")
+
+	_, err = conn.ExecuteFetch(`create table function_default (ts TIMESTAMP DEFAULT UTC_TIMESTAMP)`, 1000, true)
+	// this query fails because utc_timestamp is not supported in default clause
+	require.Error(t, err)
+
+	exec(t, conn, `create table function_default (x varchar(25) DEFAULT "check")`)
+	exec(t, conn, "drop table function_default")
+}
+
 func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) {
 	t.Helper()
 	qr := exec(t, conn, query)

go/test/endtoend/vtgate/mysql80/main_test.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2021 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package vtgate
+
+import (
+	"flag"
+	"os"
+	"testing"
+
+	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/test/endtoend/cluster"
+)
+
+var (
+	clusterInstance *cluster.LocalProcessCluster
+	vtParams        mysql.ConnParams
+	KeyspaceName    = "ks"
+	Cell            = "test"
+)
+
+func TestMain(m *testing.M) {
+	defer cluster.PanicHandler(nil)
+	flag.Parse()
+
+	exitCode := func() int {
+		clusterInstance = cluster.NewCluster(Cell, "localhost")
+		defer clusterInstance.Teardown()
+
+		// Start topo server
+		err := clusterInstance.StartTopo()
+		if err != nil {
+			return 1
+		}
+
+		// Start keyspace
+		keyspace := &cluster.Keyspace{
+			Name: KeyspaceName,
+		}
+		err = clusterInstance.StartUnshardedKeyspace(*keyspace, 0, false)
+		if err != nil {
+			return 1
+		}
+
+		clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "-enable_system_settings=true")
+		// Start vtgate
+		err = clusterInstance.StartVtgate()
+		if err != nil {
+			return 1
+		}
+		vtParams = mysql.ConnParams{
+			Host: clusterInstance.Hostname,
+			Port: clusterInstance.VtgateMySQLPort,
+		}
+		return m.Run()
+	}()
+	os.Exit(exitCode)
+}

go/test/endtoend/vtgate/mysql80/misc_test.go

@@ -0,0 +1,79 @@
+/*
+Copyright 2021 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package vtgate
+
+import (
+	"context"
+	"testing"
+
+	"vitess.io/vitess/go/test/endtoend/cluster"
+
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/sqltypes"
+)
+
+func TestFunctionInDefault(t *testing.T) {
+	defer cluster.PanicHandler(t)
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	// set the sql mode ALLOW_INVALID_DATES
+	exec(t, conn, `SET sql_mode = 'ALLOW_INVALID_DATES'`)
+
+	exec(t, conn, `create table function_default (x varchar(25) DEFAULT (TRIM(" check ")))`)
+	exec(t, conn, "drop table function_default")
+
+	exec(t, conn, `create table function_default (
+ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+dt DATETIME DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ts2 TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
+dt2 DATETIME DEFAULT CURRENT_TIMESTAMP,
+ts3 TIMESTAMP DEFAULT 0,
+dt3 DATETIME DEFAULT 0,
+ts4 TIMESTAMP DEFAULT 0 ON UPDATE CURRENT_TIMESTAMP,
+dt4 DATETIME DEFAULT 0 ON UPDATE CURRENT_TIMESTAMP,
+ts5 TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
+ts6 TIMESTAMP NULL ON UPDATE CURRENT_TIMESTAMP,
+dt5 DATETIME ON UPDATE CURRENT_TIMESTAMP,
+dt6 DATETIME NOT NULL ON UPDATE CURRENT_TIMESTAMP,
+ts7 TIMESTAMP(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),
+ts8 TIMESTAMP DEFAULT NOW(),
+ts9 TIMESTAMP DEFAULT LOCALTIMESTAMP,
+ts10 TIMESTAMP DEFAULT LOCALTIME,
+ts11 TIMESTAMP DEFAULT LOCALTIMESTAMP(),
+ts12 TIMESTAMP DEFAULT LOCALTIME()
+)`)
+	exec(t, conn, "drop table function_default")
+
+	// this query works because utc_timestamp will get parenthesised before reaching MySQL. However, this syntax is not supported in MySQL 8.0
+	exec(t, conn, `create table function_default (ts TIMESTAMP DEFAULT UTC_TIMESTAMP)`)
+	exec(t, conn, "drop table function_default")
+
+	exec(t, conn, `create table function_default (x varchar(25) DEFAULT "check")`)
+	exec(t, conn, "drop table function_default")
+}
+
+func exec(t *testing.T, conn *mysql.Conn, query string) *sqltypes.Result {
+	t.Helper()
+	qr, err := conn.ExecuteFetch(query, 1000, true)
+	require.NoError(t, err, "for query: "+query)
+	return qr
+}

go/vt/sqlparser/ast_format.go

@@ -471,7 +471,14 @@ func (ct *ColumnType) Format(buf *TrackedBuffer) {
 		}
 	}
 	if ct.Options.Default != nil {
-		buf.astPrintf(ct, " %s %v", keywordStrings[DEFAULT], ct.Options.Default)
+		buf.astPrintf(ct, " %s", keywordStrings[DEFAULT])
+		_, isLiteral := ct.Options.Default.(*Literal)
+		_, isNullVal := ct.Options.Default.(*NullVal)
+		if isLiteral || isNullVal || isExprAliasForCurrentTimeStamp(ct.Options.Default) {
+			buf.astPrintf(ct, " %v", ct.Options.Default)
+		} else {
+			buf.astPrintf(ct, " (%v)", ct.Options.Default)
+		}
 	}
 	if ct.Options.OnUpdate != nil {
 		buf.astPrintf(ct, " %s %s %v", keywordStrings[ON], keywordStrings[UPDATE], ct.Options.OnUpdate)

go/vt/sqlparser/ast_format_fast.go

@@ -672,8 +672,16 @@ func (ct *ColumnType) formatFast(buf *TrackedBuffer) {
 	if ct.Options.Default != nil {
 		buf.WriteByte(' ')
 		buf.WriteString(keywordStrings[DEFAULT])
-		buf.WriteByte(' ')
-		ct.Options.Default.formatFast(buf)
+		_, isLiteral := ct.Options.Default.(*Literal)
+		_, isNullVal := ct.Options.Default.(*NullVal)
+		if isLiteral || isNullVal || isExprAliasForCurrentTimeStamp(ct.Options.Default) {
+			buf.WriteByte(' ')
+			ct.Options.Default.formatFast(buf)
+		} else {
+			buf.WriteString(" (")
+			ct.Options.Default.formatFast(buf)
+			buf.WriteByte(')')
+		}
 	}
 	if ct.Options.OnUpdate != nil {
 		buf.WriteByte(' ')

go/vt/sqlparser/ast_funcs.go

@@ -1322,6 +1322,17 @@ func (node *ColName) CompliantName() string {
 	return node.Name.CompliantName()
 }
 
+// isExprAliasForCurrentTimeStamp returns true if the Expr provided is an alias for CURRENT_TIMESTAMP
+func isExprAliasForCurrentTimeStamp(expr Expr) bool {
+	switch node := expr.(type) {
+	case *FuncExpr:
+		return node.Name.EqualString("current_timestamp") || node.Name.EqualString("now") || node.Name.EqualString("localtimestamp") || node.Name.EqualString("localtime")
+	case *CurTimeFuncExpr:
+		return node.Name.EqualString("current_timestamp") || node.Name.EqualString("now") || node.Name.EqualString("localtimestamp") || node.Name.EqualString("localtime")
+	}
+	return false
+}
+
 // AtCount represents the '@' count in ColIdent
 type AtCount int
 

go/vt/sqlparser/parse_test.go

@@ -1169,6 +1169,15 @@ var (
 		input:      "create table `a`",
 		output:     "create table a",
 		partialDDL: true,
+	}, {
+		input:  "create table function_default (x varchar(25) default (trim(' check ')))",
+		output: "create table function_default (\n\tx varchar(25) default (trim(' check '))\n)",
+	}, {
+		input:  "create table function_default (x varchar(25) default (((trim(' check ')))))",
+		output: "create table function_default (\n\tx varchar(25) default (trim(' check '))\n)",
+	}, {
+		input:  "create table function_default3 (x bool DEFAULT (true AND false));",
+		output: "create table function_default3 (\n\tx bool default (true and false)\n)",
 	}, {
 		input:  "create table a (\n\t`a` int\n)",
 		output: "create table a (\n\ta int\n)",
@@ -2907,11 +2916,11 @@ func TestCreateTable(t *testing.T) {
 	time5 timestamp(4) default utc_timestamp(4) on update utc_timestamp(4)
 )`,
 			output: `create table t (
-	time1 timestamp default utc_timestamp(),
-	time2 timestamp default utc_timestamp(),
-	time3 timestamp default utc_timestamp() on update utc_timestamp(),
-	time4 timestamp default utc_timestamp() on update utc_timestamp(),
-	time5 timestamp(4) default utc_timestamp(4) on update utc_timestamp(4)
+	time1 timestamp default (utc_timestamp()),
+	time2 timestamp default (utc_timestamp()),
+	time3 timestamp default (utc_timestamp()) on update utc_timestamp(),
+	time4 timestamp default (utc_timestamp()) on update utc_timestamp(),
+	time5 timestamp(4) default (utc_timestamp(4)) on update utc_timestamp(4)
 )`,
 		}, {
 			// test utc_time with and without ()
@@ -2923,11 +2932,11 @@ func TestCreateTable(t *testing.T) {
 	time5 timestamp(5) default utc_time(5) on update utc_time(5)
 )`,
 			output: `create table t (
-	time1 timestamp default utc_time(),
-	time2 timestamp default utc_time(),
-	time3 timestamp default utc_time() on update utc_time(),
-	time4 timestamp default utc_time() on update utc_time(),
-	time5 timestamp(5) default utc_time(5) on update utc_time(5)
+	time1 timestamp default (utc_time()),
+	time2 timestamp default (utc_time()),
+	time3 timestamp default (utc_time()) on update utc_time(),
+	time4 timestamp default (utc_time()) on update utc_time(),
+	time5 timestamp(5) default (utc_time(5)) on update utc_time(5)
 )`,
 		}, {
 			// test utc_date with and without ()
@@ -2938,10 +2947,10 @@ func TestCreateTable(t *testing.T) {
 	time4 timestamp default utc_date() on update utc_date()
 )`,
 			output: `create table t (
-	time1 timestamp default utc_date(),
-	time2 timestamp default utc_date(),
-	time3 timestamp default utc_date() on update utc_date(),
-	time4 timestamp default utc_date() on update utc_date()
+	time1 timestamp default (utc_date()),
+	time2 timestamp default (utc_date()),
+	time3 timestamp default (utc_date()) on update utc_date(),
+	time4 timestamp default (utc_date()) on update utc_date()
 )`,
 		}, {
 			// test localtime with and without ()
@@ -2984,10 +2993,10 @@ func TestCreateTable(t *testing.T) {
 	time4 timestamp default current_date() on update current_date()
 )`,
 			output: `create table t (
-	time1 timestamp default current_date(),
-	time2 timestamp default current_date(),
-	time3 timestamp default current_date() on update current_date(),
-	time4 timestamp default current_date() on update current_date()
+	time1 timestamp default (current_date()),
+	time2 timestamp default (current_date()),
+	time3 timestamp default (current_date()) on update current_date(),
+	time4 timestamp default (current_date()) on update current_date()
 )`,
 		}, {
 			// test current_time with and without ()
@@ -2999,11 +3008,11 @@ func TestCreateTable(t *testing.T) {
 	time5 timestamp(2) default current_time(2) on update current_time(2)
 )`,
 			output: `create table t (
-	time1 timestamp default current_time(),
-	time2 timestamp default current_time(),
-	time3 timestamp default current_time() on update current_time(),
-	time4 timestamp default current_time() on update current_time(),
-	time5 timestamp(2) default current_time(2) on update current_time(2)
+	time1 timestamp default (current_time()),
+	time2 timestamp default (current_time()),
+	time3 timestamp default (current_time()) on update current_time(),
+	time4 timestamp default (current_time()) on update current_time(),
+	time5 timestamp(2) default (current_time(2)) on update current_time(2)
 )`,
 		}, {
 			input: `create table t1 (

go/vt/vtgate/planbuilder/testdata/ddl_cases.txt

@@ -413,3 +413,19 @@ Gen4 plan same as above
   }
 }
 Gen4 plan same as above
+
+# create table with function as a default value
+"create table function_default (x varchar(25) DEFAULT (TRIM(' check ')))"
+{
+  "QueryType": "DDL",
+  "Original": "create table function_default (x varchar(25) DEFAULT (TRIM(' check ')))",
+  "Instructions": {
+    "OperatorType": "DDL",
+    "Keyspace": {
+      "Name": "main",
+      "Sharded": false
+    },
+    "Query": "create table function_default (\n\tx varchar(25) default (TRIM(' check '))\n)"
+  }
+}
+Gen4 plan same as above

go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt

@@ -939,3 +939,17 @@ options:PassthroughDMLs
   "TableName": "",
   "FullQuery": "call getAllTheThings()"
 }
+
+# create table with function as a default value
+"create table function_default (x varchar(25) DEFAULT (TRIM(' check ')))"
+{
+  "PlanID": "DDL",
+  "TableName": "",
+  "Permissions": [
+    {
+      "TableName": "function_default",
+      "Role": 2
+    }
+  ],
+  "FullQuery": "create table function_default (\n\tx varchar(25) default (TRIM(' check '))\n)"
+}

test/ci_workflow_gen.go

@@ -82,6 +82,7 @@ var (
 		"xb_recovery",
 		"resharding",
 		"resharding_bytes",
+		"mysql80",
 	}
 	// TODO: currently some percona tools including xtrabackup are installed on all clusters, we can possibly optimize
 	// this by only installing them in the required clusters
@@ -90,6 +91,9 @@ var (
 		"18",
 		"24",
 	}
+	clustersRequiringUbuntu20 = []string{
+		"mysql80",
+	}
 )
 
 type unitTest struct {
@@ -99,6 +103,7 @@ type unitTest struct {
 type clusterTest struct {
 	Name, Shard                  string
 	MakeTools, InstallXtraBackup bool
+	Ubuntu20                     bool
 }
 
 func mergeBlankLines(buf *bytes.Buffer) string {
@@ -164,6 +169,13 @@ func generateClusterWorkflows() {
 				break
 			}
 		}
+		ubuntu20Clusters := canonnizeList(clustersRequiringUbuntu20)
+		for _, ubuntu20Cluster := range ubuntu20Clusters {
+			if ubuntu20Cluster == cluster {
+				test.Ubuntu20 = true
+				break
+			}
+		}
 
 		path := fmt.Sprintf("%s/cluster_endtoend_%s.yml", workflowConfigDir, cluster)
 		generateWorkflowFile(clusterTestTemplate, path, test)

test/config.json

@@ -660,6 +660,15 @@
 			"RetryMax": 0,
 			"Tags": []
 		},
+		"vtgate_mysql80": {
+			"File": "unused.go",
+			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/mysql80"],
+			"Command": [],
+			"Manual": false,
+			"Shard": "mysql80",
+			"RetryMax": 0,
+			"Tags": []
+		},
 		"vtgate_sequence": {
 			"File": "unused.go",
 			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/sequence"],

test/templates/cluster_endtoend_test.tpl

@@ -4,7 +4,7 @@ jobs:
 
   build:
     name: Run endtoend tests on {{.Name}}
-    runs-on: ubuntu-18.04
+    {{if .Ubuntu20}}runs-on: ubuntu-20.04{{else}}runs-on: ubuntu-18.04{{end}}
 
     steps:
     - name: Set up Go