Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions go/mysql/json/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
26 changes: 26 additions & 0 deletions go/test/endtoend/vtgate/queries/dml/insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}")]]`)
}
2 changes: 1 addition & 1 deletion go/test/endtoend/vtgate/queries/dml/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions go/test/endtoend/vtgate/queries/dml/sharded_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
9 changes: 8 additions & 1 deletion go/test/endtoend/vtgate/queries/dml/unsharded_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
8 changes: 8 additions & 0 deletions go/test/endtoend/vtgate/queries/dml/vschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@
"name": "hash"
}
]
},
"j_tbl": {
"column_vindexes": [
{
"column": "id",
"name": "hash"
}
]
}
}
}
31 changes: 18 additions & 13 deletions go/vt/proto/query/query.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions go/vt/sqlparser/normalizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion go/vt/sqlparser/parsed_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
37 changes: 23 additions & 14 deletions go/vt/sqlparser/parsed_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
})
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions proto/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions web/vtadmin/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion web/vtadmin/src/proto/vtadmin.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading