sql/pgwire: fix encoding of decimals with trailing 0s

The dscale is defined as number of digits (in base 10) visible after the decimal separator," so a negative dscale makes no sense and is not valid. This was preventing NPGSQL from being able to decode the binary decimals that CockroachDB was sending. Release note (bug fix): The binary encoding of decimals no longer will have negative `dscale` values. This was preventing Npgsql from being able to read some binary decimals from CockroachDB.
cockroachdb · Apr 22, 2021 · f7fe860 · f7fe860
1 parent feb357a
commit f7fe860
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 1 deletion.
diff --git a/pkg/cmd/generate-binary/main.go b/pkg/cmd/generate-binary/main.go
@@ -200,6 +200,11 @@ var inputs = map[string][]string{
 		"42.0",
 		"420000",
 		"420000.0",
+		"6000500000000.0000000",
+		"10000",
+		"800000000",
+		"9E+4",
+		"99E100",
 	},
 
 	"'%s'::float8": {

diff --git a/pkg/sql/pgwire/encoding_test.go b/pkg/sql/pgwire/encoding_test.go
@@ -156,7 +156,7 @@ func TestEncodings(t *testing.T) {
 				}
 				got := verifyLen(t)
 				if !bytes.Equal(got, tc.TextAsBinary) {
-					t.Errorf("unexpected text encoding:\n\t%q found,\n\t%q expected", got, tc.Text)
+					t.Errorf("unexpected text encoding:\n\t%q found,\n\t%q expected", got, tc.TextAsBinary)
 				}
 			}
 		})

diff --git a/pkg/sql/pgwire/testdata/encodings.json b/pkg/sql/pgwire/testdata/encodings.json
@@ -580,6 +580,41 @@
 		"TextAsBinary": [52, 50, 48, 48, 48, 48, 46, 48],
 		"Binary": [0, 1, 0, 1, 0, 0, 0, 1, 0, 42]
 	},
+	{
+		"SQL": "'6000500000000.0000000'::decimal",
+		"Oid": 1700,
+		"Text": "6000500000000.0000000",
+		"TextAsBinary": [54, 48, 48, 48, 53, 48, 48, 48, 48, 48, 48, 48, 48, 46, 48, 48, 48, 48, 48, 48, 48],
+		"Binary": [0, 2, 0, 3, 0, 0, 0, 7, 0, 6, 0, 5]
+	},
+	{
+		"SQL": "'10000'::decimal",
+		"Oid": 1700,
+		"Text": "10000",
+		"TextAsBinary": [49, 48, 48, 48, 48],
+		"Binary": [0, 1, 0, 1, 0, 0, 0, 0, 0, 1]
+	},
+	{
+		"SQL": "'800000000'::decimal",
+		"Oid": 1700,
+		"Text": "800000000",
+		"TextAsBinary": [56, 48, 48, 48, 48, 48, 48, 48, 48],
+		"Binary": [0, 1, 0, 2, 0, 0, 0, 0, 0, 8]
+	},
+	{
+		"SQL": "'9E+4'::decimal",
+		"Oid": 1700,
+		"Text": "9E+4",
+		"TextAsBinary": [57, 69, 43, 52],
+		"Binary": [0, 1, 0, 1, 0, 0, 0, 0, 0, 9]
+	},
+	{
+		"SQL": "'99E100'::decimal",
+		"Oid": 1700,
+		"Text": "9.9E+101",
+		"TextAsBinary": [57, 46, 57, 69, 43, 49, 48, 49],
+		"Binary": [0, 1, 0, 25, 0, 0, 0, 0, 0, 99]
+	},
 	{
 		"SQL": "'{-000.000,-0000021234.23246346000000,-1.2,.0,.1,.1234}'::decimal[]",
 		"Oid": 1231,

diff --git a/pkg/sql/pgwire/testdata/pgtest/decimal b/pkg/sql/pgwire/testdata/pgtest/decimal
@@ -35,3 +35,51 @@ ReadyForQuery
 {"Type":"DataRow","Values":[{"text":"0"}]}
 {"Type":"CommandComplete","CommandTag":"SELECT 1"}
 {"Type":"ReadyForQuery","TxStatus":"I"}
+
+# [0, 1, 0, 1, 0, 0, 0, 0, 0, 1] = binary 1E+4
+# [0, 1, 0, 1, 0, 0, 0, 0, 0, 10] = binary 10E+4
+send
+Parse {"Name": "s1", "Query": "SELECT 10000::decimal, $1::decimal, $2::decimal"}
+Bind {"DestinationPortal": "p1", "PreparedStatement": "s1", "ParameterFormatCodes": [1], "Parameters": [[0, 1, 0, 1, 0, 0, 0, 0, 0, 1], [0, 1, 0, 1, 0, 0, 0, 0, 0, 10]]}
+Execute {"Portal": "p1"}
+Sync
+----
+
+# CockroachDB intentionally uses exponents for decimals like 1E+4, as
+# oppposed to Postgres, which returns 10000.
+until crdb_only
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"DataRow","Values":[{"text":"10000"},{"text":"1E+4"},{"text":"1.0E+5"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+until noncrdb_only
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"DataRow","Values":[{"text":"10000"},{"text":"10000"},{"text":"100000"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+# ResultFormatCodes [1] = FormatBinary
+# [0, 1, 0, 1, 0, 0, 0, 0, 0, 1] = binary 1E+4
+# [0, 1, 0, 1, 0, 0, 0, 0, 0, 10] = binary 10E+4
+send
+Parse {"Name": "s2", "Query": "SELECT 10000::decimal, $1::decimal, $2::decimal"}
+Bind {"DestinationPortal": "p2", "PreparedStatement": "s2", "ParameterFormatCodes": [1], "Parameters": [[0, 1, 0, 1, 0, 0, 0, 0, 0, 1], [0, 1, 0, 1, 0, 0, 0, 0, 0, 10]], "ResultFormatCodes": [1,1, 1]}
+Execute {"Portal": "p2"}
+Sync
+----
+
+until
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"DataRow","Values":[{"binary":"00010001000000000001"},{"binary":"00010001000000000001"},{"binary":"0001000100000000000a"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
diff --git a/pkg/sql/pgwire/types.go b/pkg/sql/pgwire/types.go
@@ -353,6 +353,12 @@ func (b *writeBuffer) writeBinaryDatum(
 			return ndigit
 		}
 
+		// The dscale is defined as number of digits (in base 10) visible
+		// after the decimal separator, so it can't be negative.
+		if alloc.pgNum.Dscale < 0 {
+			alloc.pgNum.Dscale = 0
+		}
+
 		b.putInt32(int32(2 * (4 + alloc.pgNum.Ndigits)))
 		b.putInt16(alloc.pgNum.Ndigits)
 		b.putInt16(alloc.pgNum.Weight)