[release-18.0] JSON Encoding: Use Type_RAW for marshalling json (#16637…

…) (#16680) Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Rohit Nayak <[email protected]>
vitessio · Sep 4, 2024 · d443cf0 · d443cf0
1 parent da4aa79
commit d443cf0
Show file tree

Hide file tree

Showing 15 changed files with 156 additions and 40 deletions.
diff --git a/go/mysql/json/marshal.go b/go/mysql/json/marshal.go
@@ -173,9 +173,7 @@ func MarshalSQLValue(buf []byte) (*sqltypes.Value, error) {
 	if err != nil {
 		return nil, err
 	}
-	newVal := sqltypes.MakeTrusted(querypb.Type_JSON, jsonVal.MarshalSQLTo(nil))
-	if err != nil {
-		return nil, err
-	}
+
+	newVal := sqltypes.MakeTrusted(querypb.Type_RAW, jsonVal.MarshalSQLTo(nil))
 	return &newVal, nil
 }
diff --git a/go/test/endtoend/vtgate/queries/dml/insert_test.go b/go/test/endtoend/vtgate/queries/dml/insert_test.go
@@ -470,3 +470,29 @@ func TestMixedCases(t *testing.T) {
 	// final check count on the lookup vindex table.
 	utils.AssertMatches(t, mcmp.VtConn, "select count(*) from lkp_mixed_idx", "[[INT64(12)]]")
 }
+
+// TestInsertJson tests that selected json values are encoded correctly.
+func TestInsertJson(t *testing.T) {
+	utils.SkipIfBinaryIsBelowVersion(t, 21, "vtgate")
+	utils.SkipIfBinaryIsBelowVersion(t, 21, "vttablet")
+
+	mcmp, closer := start(t)
+	defer closer()
+
+	mcmp.Exec(`insert into j_tbl(id, jdoc) values (1, '{}'), (2, '{"a": 1, "b": 2}')`)
+	mcmp.Exec(`select * from j_tbl order by id`)
+
+	mcmp.Exec(`insert into j_tbl(id, jdoc) select 3, json_object("k", "a")`)
+	mcmp.Exec(`select * from j_tbl order by id`)
+
+	mcmp.Exec(`insert into j_tbl(id, jdoc) select 4,JSON_OBJECT(
+        'date', CAST(1629849600 AS UNSIGNED),
+        'keywordSourceId', CAST(930701976723823 AS UNSIGNED),
+        'keywordSourceVersionId', CAST(210825230433 AS UNSIGNED)
+    )`)
+	mcmp.Exec(`select * from j_tbl order by id`)
+
+	utils.Exec(t, mcmp.VtConn, `insert into uks.j_utbl(id, jdoc) select * from sks.j_tbl`)
+	utils.AssertMatches(t, mcmp.VtConn, `select * from uks.j_utbl order by id`,
+		`[[INT64(1) JSON("{}")] [INT64(2) JSON("{\"a\": 1, \"b\": 2}")] [INT64(3) JSON("{\"k\": \"a\"}")] [INT64(4) JSON("{\"date\": 1629849600, \"keywordSourceId\": 930701976723823, \"keywordSourceVersionId\": 210825230433}")]]`)
+}
diff --git a/go/test/endtoend/vtgate/queries/dml/main_test.go b/go/test/endtoend/vtgate/queries/dml/main_test.go
@@ -133,7 +133,7 @@ func start(t *testing.T) (utils.MySQLCompare, func()) {
 
 		tables := []string{
 			"s_tbl", "num_vdx_tbl", "user_tbl", "order_tbl", "oevent_tbl", "oextra_tbl",
-			"auto_tbl", "oid_vdx_tbl", "unq_idx", "nonunq_idx", "u_tbl", "mixed_tbl", "lkp_map_idx",
+			"auto_tbl", "oid_vdx_tbl", "unq_idx", "nonunq_idx", "u_tbl", "mixed_tbl", "lkp_map_idx", "j_tbl", "j_utbl",
 		}
 		for _, table := range tables {
 			// TODO (@frouioui): following assertions produce different results between MySQL and Vitess

diff --git a/go/test/endtoend/vtgate/queries/dml/sharded_schema.sql b/go/test/endtoend/vtgate/queries/dml/sharded_schema.sql
@@ -86,3 +86,10 @@ create table lkp_mixed_idx
     keyspace_id varbinary(20),
     primary key (lkp_key)
 ) Engine = InnoDB;
+
+create table j_tbl
+(
+    id  bigint,
+    jdoc json,
+    primary key (id)
+) Engine = InnoDB;
diff --git a/go/test/endtoend/vtgate/queries/dml/unsharded_schema.sql b/go/test/endtoend/vtgate/queries/dml/unsharded_schema.sql
@@ -34,4 +34,11 @@ values (0, 1, 1000);
 insert into auto_seq(id, next_id, cache)
 values (0, 666, 1000);
 insert into mixed_seq(id, next_id, cache)
-values (0, 1, 1000);
+values (0, 1, 1000);
+
+create table j_utbl
+(
+    id  bigint,
+    jdoc json,
+    primary key (id)
+) Engine = InnoDB;
diff --git a/go/test/endtoend/vtgate/queries/dml/vschema.json b/go/test/endtoend/vtgate/queries/dml/vschema.json
@@ -188,6 +188,14 @@
           "name": "hash"
         }
       ]
+    },
+    "j_tbl": {
+      "column_vindexes": [
+        {
+          "column": "id",
+          "name": "hash"
+        }
+      ]
     }
   }
 }
diff --git a/go/vt/proto/query/query.pb.go b/go/vt/proto/query/query.pb.go
diff --git a/go/vt/sqlparser/normalizer_test.go b/go/vt/sqlparser/normalizer_test.go
@@ -209,6 +209,23 @@ func TestNormalize(t *testing.T) {
 		outbv: map[string]*querypb.BindVariable{
 			"v1": sqltypes.HexNumBindVariable([]byte("0x03")),
 		},
+	}, {
+		// json value in insert
+		in:      "insert into t values ('{\"k\", \"v\"}')",
+		outstmt: "insert into t values (:bv1 /* VARCHAR */)",
+		outbv: map[string]*querypb.BindVariable{
+			"bv1": sqltypes.StringBindVariable("{\"k\", \"v\"}"),
+		},
+	}, {
+		// json function in insert
+		in:      "insert into t values (JSON_OBJECT('_id', 27, 'name', 'carrot'))",
+		outstmt: "insert into t values (json_object(:bv1 /* VARCHAR */, :bv2 /* INT64 */, :bv3 /* VARCHAR */, :bv4 /* VARCHAR */))",
+		outbv: map[string]*querypb.BindVariable{
+			"bv1": sqltypes.StringBindVariable("_id"),
+			"bv2": sqltypes.Int64BindVariable(27),
+			"bv3": sqltypes.StringBindVariable("name"),
+			"bv4": sqltypes.StringBindVariable("carrot"),
+		},
 	}, {
 		// ORDER BY column_position
 		in:      "select a, b from t order by 1 asc",

diff --git a/go/vt/sqlparser/parsed_query.go b/go/vt/sqlparser/parsed_query.go
@@ -176,7 +176,7 @@ func EncodeValue(buf *strings.Builder, value *querypb.BindVariable) {
 			sqltypes.ProtoToValue(bv).EncodeSQLStringBuilder(buf)
 		}
 		buf.WriteByte(')')
-	case querypb.Type_JSON:
+	case querypb.Type_RAW:
 		v, _ := sqltypes.BindVariableToValue(value)
 		buf.Write(v.Raw())
 	default:

diff --git a/go/vt/sqlparser/parsed_query_test.go b/go/vt/sqlparser/parsed_query_test.go
@@ -20,6 +20,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"vitess.io/vitess/go/sqltypes"
 	querypb "vitess.io/vitess/go/vt/proto/query"
 
@@ -79,6 +81,14 @@ func TestGenerateQuery(t *testing.T) {
 				"vals": sqltypes.TestBindVariable([]any{1, "aa"}),
 			},
 			output: "select * from a where id in (1, 'aa')",
+		}, {
+			desc:  "json bindvar and raw bindvar",
+			query: "insert into t values (:v1, :v2)",
+			bindVars: map[string]*querypb.BindVariable{
+				"v1": sqltypes.ValueBindVariable(sqltypes.MakeTrusted(querypb.Type_JSON, []byte(`{"key": "value"}`))),
+				"v2": sqltypes.ValueBindVariable(sqltypes.MakeTrusted(querypb.Type_RAW, []byte(`json_object("k", "v")`))),
+			},
+			output: `insert into t values ('{\"key\": \"value\"}', json_object("k", "v"))`,
 		}, {
 			desc:  "list bind vars 0 arguments",
 			query: "select * from a where id in ::vals",
@@ -136,20 +146,19 @@ func TestGenerateQuery(t *testing.T) {
 	}
 
 	for _, tcase := range tcases {
-		tree, err := Parse(tcase.query)
-		if err != nil {
-			t.Errorf("parse failed for %s: %v", tcase.desc, err)
-			continue
-		}
-		buf := NewTrackedBuffer(nil)
-		buf.Myprintf("%v", tree)
-		pq := buf.ParsedQuery()
-		bytes, err := pq.GenerateQuery(tcase.bindVars, tcase.extras)
-		if err != nil {
-			assert.Equal(t, tcase.output, err.Error())
-		} else {
-			assert.Equal(t, tcase.output, string(bytes))
-		}
+		t.Run(tcase.query, func(t *testing.T) {
+			tree, err := Parse(tcase.query)
+			require.NoError(t, err)
+			buf := NewTrackedBuffer(nil)
+			buf.Myprintf("%v", tree)
+			pq := buf.ParsedQuery()
+			bytes, err := pq.GenerateQuery(tcase.bindVars, tcase.extras)
+			if err != nil {
+				assert.Equal(t, tcase.output, err.Error())
+			} else {
+				assert.Equal(t, tcase.output, bytes)
+			}
+		})
 	}
 }
 

diff --git a/java/jdbc/src/test/java/io/vitess/jdbc/FieldWithMetadataTest.java b/java/jdbc/src/test/java/io/vitess/jdbc/FieldWithMetadataTest.java
@@ -16,6 +16,9 @@
 
 package io.vitess.jdbc;
 
+import java.util.Set;
+import java.util.EnumSet;
+
 import io.vitess.proto.Query;
 import io.vitess.util.MysqlDefs;
 import io.vitess.util.charset.CharsetMapping;
@@ -274,6 +277,16 @@ public void testNumericAndDateTimeEncoding() throws SQLException {
     }
   }
 
+  // Define the types to skip
+  Set<Query.Type> typesToSkip = EnumSet.of(
+          Query.Type.UNRECOGNIZED,
+          Query.Type.EXPRESSION,
+          Query.Type.HEXVAL,
+          Query.Type.HEXNUM,
+          Query.Type.BITNUM,
+          Query.Type.RAW
+  );
+
   @Test
   public void testPrecisionAdjustFactor() throws SQLException {
     VitessConnection conn = getVitessConnection();
@@ -294,7 +307,8 @@ public void testPrecisionAdjustFactor() throws SQLException {
 
     conn.setIncludedFields(Query.ExecuteOptions.IncludedFields.TYPE_AND_NAME);
     for (Query.Type type : Query.Type.values()) {
-      if (type == Query.Type.UNRECOGNIZED || type == Query.Type.EXPRESSION || type == Query.Type.HEXVAL || type == Query.Type.HEXNUM || type == Query.Type.BITNUM) {
+      // Skip if the type is in the set
+      if (typesToSkip.contains(type)) {
         continue;
       }
 

diff --git a/proto/query.proto b/proto/query.proto
@@ -215,6 +215,8 @@ enum Type {
   // BITNUM specifies a base 2 binary type (unquoted varbinary).
   // Properties: 34, IsText.
   BITNUM = 4130;
+  // RAW specifies a type which won't be quoted but the value used as-is while encoding.
+  RAW = 2084;
 }
 
 // Value represents a typed value.

diff --git a/web/vtadmin/package-lock.json b/web/vtadmin/package-lock.json
diff --git a/web/vtadmin/src/proto/vtadmin.d.ts b/web/vtadmin/src/proto/vtadmin.d.ts