sql/pgwire: fix encoding of int4 and bpchar in tuples #61013
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are a few minor differences: dataTypeSize is different for tuples, and PG does not show seconds offset for times if it is zero. Release note: None
Release note (bug fix): Integers inside of tuples were not being encoded properly when using the binary format for retrieving data. This is now fixed, and the proper integer width is reported.
|
This change is |
Release note (bug fix): Blank-padded chars (e.g. CHAR(3)) were not being encoded correctly when returning results to the client. Now they correctly include blank-padding when appropriate.
rafiss
force-pushed
the
tuple-binary-encoding
branch
from
98df18a to
f970cac
Compare
otan
reviewed
Feb 23, 2021
pkg/sql/pgwire/types.go
Outdated
| @@ -422,7 +410,7 @@ func (b *writeBuffer) writeBinaryDatum( | |||
| b.writeLengthPrefixedString(v.LogicalRep) | |||
|
|
|||
| case *tree.DString: | |||
| b.writeLengthPrefixedString(resolveBlankPaddedChar(string(*v), t)) | |||
| b.writeLengthPrefixedString(tree.ResolveBlankPaddedChar(string(*v), t)) | |||
|
|
|||
| case *tree.DCollatedString: | |||
| b.writeLengthPrefixedString(v.Contents) | |||
There was a problem hiding this comment.
do we need to resolve blank padded chars for collated strings too?
There was a problem hiding this comment.
yeah i think i should add it
Release note (bug fix): Collated strings were not encoded with the proper type OID when sending results to the client if the OID was for the `char` type. This is now fixed.
rafiss
force-pushed
the
tuple-binary-encoding
branch
from
e561c73 to
837efef
Compare
otan
approved these changes
Feb 23, 2021
| @@ -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 | |||
There was a problem hiding this comment.
treating collated strings differently to postgres has truly been a pain :(
There was a problem hiding this comment.
yeah the gift that keeps giving
|
tftr! bors r=otan |
|
Build failed (retrying...): |
|
bors r=otan |
|
Already running a review |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #58069
This includes 3 commits:
sql/pgwire: make PGTest pass against PostgresSQL
There are a few minor differences: dataTypeSize is different for tuples,
and PG does not show seconds offset for times if it is zero.
sql/pgwire: fix binary encoding of ints in tuples
sql/pgwire: fix text encoding of bpchar in tuples
sql/pgwire: fix encoding of collated strings
Release note (bug fix): Integers inside of tuples were not being encoded
properly when using the binary format for retrieving data. This is now
fixed, and the proper integer width is reported.
Release note (bug fix): Blank-padded chars (e.g. CHAR(3)) were not being
encoded correctly when returning results to the client. Now they
correctly include blank-padding when appropriate.
Release note (bug fix): Collated strings were not encoded with the
proper type OID when sending results to the client if the OID was
for the
chartype. This is now fixed.