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.

Sign up for GitHub

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

sql/pgwire: fix encoding of int4 and bpchar in tuples #61013

Merged
merged 4 commits into from
Feb 24, 2021
Merged

sql/pgwire: fix encoding of int4 and bpchar in tuples #61013

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83dc026
sql/pgwire: make PGTest pass against PostgresSQL
rafiss Feb 23, 2021
5d2c2cb
sql/pgwire: fix binary encoding of ints in tuples
rafiss Feb 23, 2021
f970cac
sql/pgwire: fix text encoding of bpchar in tuples
rafiss Feb 23, 2021
837efef
sql/pgwire: fix encoding of collated strings
rafiss Feb 23, 2021
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
10 changes: 10 additions & 0 deletions pkg/cmd/generate-binary/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,11 @@ var inputs = map[string][]string{
"hello123",
},

`'%s'::char(8) COLLATE "en_US"`: {
"hello",
"hello123",
},

"'%s'::timestamp": {
"1999-01-08 04:05:06+00",
"1999-01-08 04:05:06+00:00",
Expand Down Expand Up @@ -515,5 +520,10 @@ var inputs = map[string][]string{
`1::char(2)`,
`1::char(1)`,
`1::varchar(4)`,
`1::text`,
`1::char(2) COLLATE "en_US"`,
`1::char(1) COLLATE "en_US"`,
`1::varchar(4) COLLATE "en_US"`,
`1::text COLLATE "en_US"`,
},
}
5 changes: 5 additions & 0 deletions pkg/sql/pgwire/encoding_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ func TestEncodings(t *testing.T) {
case *tree.DTuple:
// Unsupported.
continue
case *tree.DCollatedString:
// Decoding collated strings is unsupported by this test. The encoded
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

treating collated strings differently to postgres has truly been a pain :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah the gift that keeps giving

// value is the same as a normal string, so decoding it turns it into
// a DString.
continue
}
for code, value := range map[pgwirebase.FormatCode][]byte{
pgwirebase.FormatText: tc.TextAsBinary,
Expand Down
49 changes: 49 additions & 0 deletions pkg/sql/pgwire/testdata/encodings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@
"TextAsBinary": [104, 101, 108, 108, 111, 49, 50, 51],
"Binary": [104, 101, 108, 108, 111, 49, 50, 51]
},
{
"SQL": "'hello'::char(8) COLLATE \"en_US\"",
"Oid": 1042,
"Text": "hello ",
"TextAsBinary": [104, 101, 108, 108, 111, 32, 32, 32],
"Binary": [104, 101, 108, 108, 111, 32, 32, 32]
},
{
"SQL": "'hello123'::char(8) COLLATE \"en_US\"",
"Oid": 1042,
"Text": "hello123",
"TextAsBinary": [104, 101, 108, 108, 111, 49, 50, 51],
"Binary": [104, 101, 108, 108, 111, 49, 50, 51]
},
{
"SQL": "'1999-01-08'::date",
"Oid": 1082,
Expand Down Expand Up @@ -1721,6 +1735,41 @@
"TextAsBinary": [40, 49, 44, 41],
"Binary": [0, 0, 0, 2, 0, 0, 4, 19, 0, 0, 0, 1, 49, 0, 0, 2, 193, 255, 255, 255, 255]
},
{
"SQL": "(1::text,null)",
"Oid": 2249,
"Text": "(1,)",
"TextAsBinary": [40, 49, 44, 41],
"Binary": [0, 0, 0, 2, 0, 0, 0, 25, 0, 0, 0, 1, 49, 0, 0, 2, 193, 255, 255, 255, 255]
},
{
"SQL": "(1::char(2) COLLATE \"en_US\",null)",
"Oid": 2249,
"Text": "(\"1 \",)",
"TextAsBinary": [40, 34, 49, 32, 34, 44, 41],
"Binary": [0, 0, 0, 2, 0, 0, 4, 18, 0, 0, 0, 2, 49, 32, 0, 0, 2, 193, 255, 255, 255, 255]
},
{
"SQL": "(1::char(1) COLLATE \"en_US\",null)",
"Oid": 2249,
"Text": "(1,)",
"TextAsBinary": [40, 49, 44, 41],
"Binary": [0, 0, 0, 2, 0, 0, 4, 18, 0, 0, 0, 1, 49, 0, 0, 2, 193, 255, 255, 255, 255]
},
{
"SQL": "(1::varchar(4) COLLATE \"en_US\",null)",
"Oid": 2249,
"Text": "(1,)",
"TextAsBinary": [40, 49, 44, 41],
"Binary": [0, 0, 0, 2, 0, 0, 4, 19, 0, 0, 0, 1, 49, 0, 0, 2, 193, 255, 255, 255, 255]
},
{
"SQL": "(1::text COLLATE \"en_US\",null)",
"Oid": 2249,
"Text": "(1,)",
"TextAsBinary": [40, 49, 44, 41],
"Binary": [0, 0, 0, 2, 0, 0, 0, 25, 0, 0, 0, 1, 49, 0, 0, 2, 193, 255, 255, 255, 255]
},
{
"SQL": "B''",
"Oid": 1560,
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/pgwire/testdata/pgtest/tuple
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ ReadyForQuery
# 80 = ASCII 'P' for Portal
# ResultFormatCodes [1] = FormatBinary
send
Parse {"Name": "s2", "Query": "SELECT ('a'::text, 'b'::varchar(4), 'c'::char(1), 'd'::char(2), 'e'::\"char\") AS row"}
Parse {"Name": "s2", "Query": "SELECT ('a'::text, 'b'::varchar(4), 'c'::char(1), 'd'::char(2), 'e'::\"char\", 'f'::char(3) COLLATE \"en_US\") AS row"}
Bind {"DestinationPortal": "p2", "PreparedStatement": "s2", "ResultFormatCodes": [1]}
Describe {"ObjectType": 80, "Name": "p2"}
Execute {"Portal": "p2"}
Expand All @@ -39,14 +39,14 @@ ReadyForQuery
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"RowDescription","Fields":[{"Name":"row","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":2249,"DataTypeSize":0,"TypeModifier":-1,"Format":1}]}
{"Type":"DataRow","Values":[{"binary":"0000000500000019000000016100000413000000016200000412000000016300000412000000026420000000120000000165"}]}
{"Type":"DataRow","Values":[{"binary":"00000006000000190000000161000004130000000162000004120000000163000004120000000264200000001200000001650000041200000003662020"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# 80 = ASCII 'P' for Portal
# ResultFormatCodes [0] = FormatText
send
Parse {"Name": "s3", "Query": "SELECT ('a'::text, 'b'::varchar(4), 'c'::char(1), 'd'::char(2), 'e'::\"char\") AS row"}
Parse {"Name": "s3", "Query": "SELECT ('a'::text, 'b'::varchar(4), 'c'::char(1), 'd'::char(2), 'e'::\"char\", 'f'::char(3) COLLATE \"en_US\") AS row"}
Bind {"DestinationPortal": "p3", "PreparedStatement": "s3", "ResultFormatCodes": [0]}
Describe {"ObjectType": 80, "Name": "p3"}
Execute {"Portal": "p3"}
Expand All @@ -61,6 +61,6 @@ ReadyForQuery
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"RowDescription","Fields":[{"Name":"row","TableOID":0,"TableAttributeNumber":0,"DataTypeOID":2249,"DataTypeSize":0,"TypeModifier":-1,"Format":0}]}
{"Type":"DataRow","Values":[{"text":"(a,b,c,\"d \",e)"}]}
{"Type":"DataRow","Values":[{"text":"(a,b,c,\"d \",e,\"f \")"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"I"}
4 changes: 2 additions & 2 deletions pkg/sql/pgwire/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (b *writeBuffer) writeTextDatum(
b.writeLengthPrefixedString(tree.ResolveBlankPaddedChar(string(*v), t))

case *tree.DCollatedString:
b.writeLengthPrefixedString(v.Contents)
b.writeLengthPrefixedString(tree.ResolveBlankPaddedChar(v.Contents, t))

case *tree.DDate:
b.textFormatter.FormatNode(v)
Expand Down Expand Up @@ -413,7 +413,7 @@ func (b *writeBuffer) writeBinaryDatum(
b.writeLengthPrefixedString(tree.ResolveBlankPaddedChar(string(*v), t))

case *tree.DCollatedString:
b.writeLengthPrefixedString(v.Contents)
b.writeLengthPrefixedString(tree.ResolveBlankPaddedChar(v.Contents, t))

case *tree.DTimestamp:
b.putInt32(8)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sem/tree/pgwire_encode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (d *DTuple) pgwireFormat(ctx *FmtCtx) {
comma := ""
for i, v := range d.D {
ctx.WriteString(comma)
t := d.typ.TupleContents()[i]
t := d.ResolvedType().TupleContents()[i]
switch dv := UnwrapDatum(nil, v).(type) {
case dNull:
case *DString:
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/sem/tree/type_check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,11 @@ func (expr *CollateExpr) TypeCheck(
return nil, err
}
t := subExpr.ResolvedType()
if types.IsStringType(t) || t.Family() == types.UnknownFamily {
if types.IsStringType(t) {
expr.Expr = subExpr
expr.typ = types.MakeCollatedString(t, expr.Locale)
return expr, nil
} else if t.Family() == types.UnknownFamily {
expr.Expr = subExpr
expr.typ = types.MakeCollatedString(types.String, expr.Locale)
return expr, nil
Expand Down