Fix accepting bind variables in time related function calls. (#14763)

Signed-off-by: Manan Gupta <[email protected]>
vitessio · Dec 13, 2023 · 8316759 · 8316759
1 parent b440407
commit 8316759
Show file tree

Hide file tree

Showing 17 changed files with 3,207 additions and 3,141 deletions.
diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go
@@ -64,6 +64,16 @@ func TestBitVals(t *testing.T) {
 	mcmp.AssertMatchesNoCompare(`select 1 + b'1001', 2 + 0x9, 3 + B'010011011010' from t1`, `[[INT64(10) UINT64(11) INT64(1245)]]`, `[[UINT64(10) UINT64(11) UINT64(1245)]]`)
 }
 
+// TestTimeFunctionWithPrecision tests that inserting data with NOW(1) works as intended.
+func TestTimeFunctionWithPrecision(t *testing.T) {
+	mcmp, closer := start(t)
+	defer closer()
+
+	mcmp.Exec("insert into t1(id1, id2) values (1, NOW(1))")
+	mcmp.Exec("insert into t1(id1, id2) values (2, NOW(2))")
+	mcmp.Exec("insert into t1(id1, id2) values (3, NOW())")
+}
+
 func TestHexVals(t *testing.T) {
 	mcmp, closer := start(t)
 	defer closer()
@@ -180,6 +190,7 @@ func TestHighNumberOfParams(t *testing.T) {
 }
 
 func TestPrepareStatements(t *testing.T) {
+	utils.SkipIfBinaryIsBelowVersion(t, 17, "vtgate")
 	mcmp, closer := start(t)
 	defer closer()
 

diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go
@@ -2483,7 +2483,7 @@ type (
 	// supported functions are documented in the grammar
 	CurTimeFuncExpr struct {
 		Name IdentifierCI
-		Fsp  int // fractional seconds precision, integer from 0 to 6 or an Argument
+		Fsp  Expr // fractional seconds precision, integer from 0 to 6 or an Argument
 	}
 
 	// ExtractedSubquery is a subquery that has been extracted from the original AST

diff --git a/go/vt/sqlparser/ast_clone.go b/go/vt/sqlparser/ast_clone.go
diff --git a/go/vt/sqlparser/ast_copy_on_rewrite.go b/go/vt/sqlparser/ast_copy_on_rewrite.go
diff --git a/go/vt/sqlparser/ast_equals.go b/go/vt/sqlparser/ast_equals.go
diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go
@@ -1529,8 +1529,8 @@ func (node *WeightStringFuncExpr) Format(buf *TrackedBuffer) {
 
 // Format formats the node.
 func (node *CurTimeFuncExpr) Format(buf *TrackedBuffer) {
-	if node.Fsp > 0 {
-		buf.astPrintf(node, "%#s(%d)", node.Name.String(), node.Fsp)
+	if node.Fsp != nil {
+		buf.astPrintf(node, "%#s(%v)", node.Name.String(), node.Fsp)
 	} else {
 		buf.astPrintf(node, "%#s()", node.Name.String())
 	}

diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go
diff --git a/go/vt/sqlparser/ast_rewrite.go b/go/vt/sqlparser/ast_rewrite.go
diff --git a/go/vt/sqlparser/ast_visit.go b/go/vt/sqlparser/ast_visit.go
diff --git a/go/vt/sqlparser/cached_size.go b/go/vt/sqlparser/cached_size.go
diff --git a/go/vt/sqlparser/normalizer.go b/go/vt/sqlparser/normalizer.go
@@ -70,6 +70,10 @@ func (nz *normalizer) walkStatementUp(cursor *Cursor) bool {
 	if !isLiteral {
 		return true
 	}
+	_, isCurTimeFunc := cursor.Parent().(*CurTimeFuncExpr)
+	if isCurTimeFunc {
+		return true
+	}
 	nz.convertLiteral(node, cursor)
 	return nz.err == nil // only continue if we haven't found any errors
 }
@@ -135,6 +139,8 @@ func (nz *normalizer) walkDownSelect(node, parent SQLNode) bool {
 	case *ConvertType:
 		// we should not rewrite the type description
 		return false
+	case *CurTimeFuncExpr:
+		return false
 	}
 	return nz.err == nil // only continue if we haven't found any errors
 }

diff --git a/go/vt/sqlparser/normalizer_test.go b/go/vt/sqlparser/normalizer_test.go
@@ -379,6 +379,18 @@ func TestNormalize(t *testing.T) {
 			"v1": sqltypes.HexValBindVariable([]byte("x'31'")),
 			"v2": sqltypes.Int64BindVariable(31),
 		},
+	}, {
+		// we don't want to replace literals in cur time function calls.
+		in:      `insert into t1(id2) values (NOW(1))`,
+		outstmt: `insert into t1(id2) values (now(1))`,
+		outbv:   map[string]*querypb.BindVariable{},
+	}, {
+		// we don't want to replace literals in cur time function calls.
+		in:      `select (select now(2)) from (select 1 from dual where NOW(1) < 2) as t`,
+		outstmt: `select (select now(2) from dual) from (select 1 from dual where now(1) < :bv1 /* INT64 */) as t`,
+		outbv: map[string]*querypb.BindVariable{
+			"bv1": sqltypes.Int64BindVariable(2),
+		},
 	}}
 	for _, tc := range testcases {
 		t.Run(tc.in, func(t *testing.T) {

diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go
@@ -1024,6 +1024,8 @@ var (
 		input: "select /* utc_timestamp as func */ utc_timestamp() from t",
 	}, {
 		input: "select /* utc_timestamp with fsp */ utc_timestamp(1) from t",
+	}, {
+		input: "select /* utc_timestamp with fsp */ utc_timestamp(:val1) from t",
 	}, {
 		input: "select /* utc_time */ utc_time() from t",
 	}, {