Skip to content

Commit

Permalink
Merge #94705
Browse files Browse the repository at this point in the history
94705: sql/pgwire: support for decoding JSON[] OID by aliasing to JSONB[] OID r=ZhouXing19 a=ZhouXing19

Previously, with the parameter type oid (199) for JSON[], the pgwire parse message
will fail to be executed with a `unknown oid type: 199`. We now aliasing it
to the one for `JSONB[]`.

This PR also includes a test revealing #95434.

This commit follows the changes in #88379.

fixes #90839

Release note (sql change): The pgwire protocol implementation can
now accept arguments of the JSON[] type (oid=199). Previously, it could
only accept JSONB[] (oid=3804). Internally, JSON[] and JSONB[] values are
still identical, so this change only affects how the values are received
over the wire protocol.

Co-authored-by: Jane Xing <[email protected]>
  • Loading branch information
craig[bot] and ZhouXing19 committed Jan 19, 2023
2 parents 9735954 + 78ffc00 commit 0bc78ed
Show file tree
Hide file tree
Showing 11 changed files with 725 additions and 18 deletions.
9 changes: 6 additions & 3 deletions pkg/sql/conn_executor_prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,11 +417,14 @@ func (ex *connExecutor) execBind(
} else {
typ, ok := types.OidToType[t]
if !ok {
// These special cases for json, json[] is here so we can
// support decoding parameters with oid=json/json[] without
// adding full support for these type.
// TODO(sql-exp): Remove this if we support JSON.
if t == oid.T_json {
// This special case is here so we can support decoding parameters
// with oid=json without adding full support for the JSON type.
// TODO(sql-exp): Remove this if we support JSON.
typ = types.Json
} else if t == oid.T__json {
typ = types.JSONArrayForDecodingOnly
} else {
var err error
typ, err = ex.planner.ResolveTypeByOID(ctx, t)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/json
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ SELECT * from foo where bar->'x' = '{}'
query T
SELECT array_agg(bar ORDER BY pk) FROM foo
----
{"'{\"a\": \"b\"}'","'[1, 2, 3]'","\"hello\"",1.000,true,false,NULL,"'{\"x\": [1, 2, 3]}'","'{\"x\": {\"y\": \"z\"}}'"}
{"{\"a\": \"b\"}","[1, 2, 3]","\"hello\"",1.000,true,false,NULL,"{\"x\": [1, 2, 3]}","{\"x\": {\"y\": \"z\"}}"}

statement ok
DELETE FROM foo
Expand Down
11 changes: 8 additions & 3 deletions pkg/sql/pgwire/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -973,13 +973,18 @@ func (c *conn) handleParse(
sqlTypeHints[i] = nil
continue
}
// This special case for json, json[] is here so we can support decoding
// parameters with oid=json/json[] without adding full support for these
// type.
// TODO(sql-exp): Remove this if we support JSON.
if t == oid.T_json {
// This special case is here so we can support decoding parameters
// with oid=json without adding full support for the JSON type.
// TODO(sql-exp): Remove this if we support JSON.
sqlTypeHints[i] = types.Json
continue
}
if t == oid.T__json {
sqlTypeHints[i] = types.JSONArrayForDecodingOnly
continue
}
v, ok := types.OidToType[t]
if !ok {
err := pgwirebase.NewProtocolViolationErrorf("unknown oid type: %v", t)
Expand Down
28 changes: 28 additions & 0 deletions pkg/sql/pgwire/pgwirebase/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,34 @@ func DecodeDatum(
return nil, err
}
return tree.ParseDJSON(string(b))
case oid.T__json, oid.T__jsonb:
var arr pgtype.JSONBArray
if err := arr.DecodeText(nil, b); err != nil {
return nil, tree.MakeParseError(string(b), typ, err)
}
if arr.Status != pgtype.Present {
return tree.DNull, nil
}
if err := validateArrayDimensions(len(arr.Dimensions), len(arr.Elements)); err != nil {
return nil, err
}
out := tree.NewDArray(types.Jsonb)
var d tree.Datum
var err error
for _, v := range arr.Elements {
if v.Status != pgtype.Present {
d = tree.DNull
} else {
d, err = tree.ParseDJSON(string(v.Bytes))
if err != nil {
return nil, err
}
}
if err := out.Append(d); err != nil {
return nil, err
}
}
return out, nil
case oid.T_tsquery:
ret, err := tsearch.ParseTSQuery(string(b))
if err != nil {
Expand Down
65 changes: 65 additions & 0 deletions pkg/sql/pgwire/testdata/pgtest/json
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,68 @@ ReadyForQuery
{"Type":"DataRow","Values":[{"binary":"017b226b6579223a202276616c227d"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# Test formatting for spaces between key and value in a json.
# https://github.com/cockroachdb/cockroach/issues/95434.
send
Parse {"Query": "SELECT $1::JSON"}
Bind {"ParameterFormatCodes": [0], "Parameters": [{"text":"{\"key\": \"val\"}"}]}
Describe {"ObjectType": "S"}
Execute
Sync
----

# PG's format output follows the input.
until noncrdb_only
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"ParameterDescription","ParameterOIDs":[114]}
{"Type":"RowDescription","Fields":[{"Name":"json","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":114,"DataTypeSize":-1,"TypeModifier":-1,"Format":0}]}
{"Type":"DataRow","Values":[{"text":"{\"key\": \"val\"}"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# CRDB has a fixed len of space between the key and value when formatting the json.
until crdb_only
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"ParameterDescription","ParameterOIDs":[3802]}
{"Type":"RowDescription","Fields":[{"Name":"jsonb","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":3802,"DataTypeSize":-1,"TypeModifier":-1,"Format":0}]}
{"Type":"DataRow","Values":[{"text":"{\"key\": \"val\"}"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

send
Parse {"Query": "SELECT $1::JSON"}
Bind {"ParameterFormatCodes": [0], "Parameters": [{"text":"{\"key\":\"val\"}"}]}
Describe {"ObjectType": "S"}
Execute
Sync
----

until noncrdb_only
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"ParameterDescription","ParameterOIDs":[114]}
{"Type":"RowDescription","Fields":[{"Name":"json","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":114,"DataTypeSize":-1,"TypeModifier":-1,"Format":0}]}
{"Type":"DataRow","Values":[{"text":"{\"key\":\"val\"}"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# CRDB has a fixed len of space between the key and value when formatting the json.
until crdb_only
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"ParameterDescription","ParameterOIDs":[3802]}
{"Type":"RowDescription","Fields":[{"Name":"jsonb","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":3802,"DataTypeSize":-1,"TypeModifier":-1,"Format":0}]}
{"Type":"DataRow","Values":[{"text":"{\"key\": \"val\"}"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}
Loading

0 comments on commit 0bc78ed

Please sign in to comment.