pgwire: handle decoding Geometry/Geography in binary

Release note (bug fix): Previously, CockroachDB would error when receiving Geometry/Geography using binary parameters. This is now resolved.
cockroachdb · Dec 14, 2022 · 6ed8b8a · 6ed8b8a
1 parent 3d4c4c4
commit 6ed8b8a
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 3 deletions.
diff --git a/pkg/cmd/generate-binary/BUILD.bazel b/pkg/cmd/generate-binary/BUILD.bazel
@@ -8,6 +8,7 @@ go_library(
     visibility = ["//visibility:private"],
     deps = [
         "//pkg/cmd/cmp-protocol/pgconnect",
+        "//pkg/sql/oidext",
         "//pkg/sql/pgwire/pgwirebase",
     ],
 )

diff --git a/pkg/cmd/generate-binary/main.go b/pkg/cmd/generate-binary/main.go
@@ -32,9 +32,11 @@ import (
 	"math"
 	"os"
 	"sort"
+	"strings"
 	"text/template"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/cmp-protocol/pgconnect"
+	"github.com/cockroachdb/cockroach/pkg/sql/oidext"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase"
 )
 
@@ -58,7 +60,7 @@ func main() {
 	var data []entry
 	ctx := context.Background()
 
-	stmts := os.Args[1:]
+	stmts := flag.Args()
 
 	if len(stmts) == 0 {
 		// Sort hard coded inputs by key name.
@@ -92,6 +94,13 @@ func main() {
 		if err != nil {
 			log.Fatalf("oid: %s: %v", sql, err)
 		}
+		// Hardcode OIDs for geometry and geography.
+		if strings.HasSuffix(expr, "::geometry") {
+			id = []byte(fmt.Sprintf("%d", oidext.T_geometry))
+		}
+		if strings.HasSuffix(expr, "::geography") {
+			id = []byte(fmt.Sprintf("%d", oidext.T_geography))
+		}
 		data = append(data, entry{
 			SQL:          expr,
 			Oid:          string(id),
@@ -364,6 +373,15 @@ var inputs = map[string][]string{
 		"23:59:59.999999",
 	},
 
+	"'%s'::geometry": {
+		"SRID=4326;POINT EMPTY",
+		"GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(1 1, 2 2))",
+	},
+	"'%s'::geography": {
+		"SRID=4326;POINT EMPTY",
+		"GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(1 1, 2 2))",
+	},
+
 	"'%s'::interval": {
 		"10y10mon",
 		"10mon10d",

diff --git a/pkg/sql/pgwire/encoding_test.go b/pkg/sql/pgwire/encoding_test.go
@@ -126,8 +126,8 @@ func readEncodingTests(t testing.TB) []*encodingTest {
 // and ensure they are identical to what Postgres produces. Regenerate that
 // file by:
 //
-//	Starting a postgres server on :5432 then running:
-//	cd pkg/cmd/generate-binary; go run main.go > ../../sql/pgwire/testdata/encodings.json
+//	Starting a postgres server with PostGIS installed on :5432 then running:
+//	bazel run pkg/cmd/generate-binary > ../../sql/pgwire/testdata/encodings.json
 func TestEncodings(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)

diff --git a/pkg/sql/pgwire/pgwirebase/BUILD.bazel b/pkg/sql/pgwire/pgwirebase/BUILD.bazel
@@ -20,6 +20,7 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/geo",
         "//pkg/settings",
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/lex",

diff --git a/pkg/sql/pgwire/pgwirebase/encoding.go b/pkg/sql/pgwire/pgwirebase/encoding.go
@@ -24,6 +24,7 @@ import (
 	"unicode/utf8"
 	"unsafe"
 
+	"github.com/cockroachdb/cockroach/pkg/geo"
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/sql/lex"
 	"github.com/cockroachdb/cockroach/pkg/sql/oidext"
@@ -813,6 +814,18 @@ func DecodeDatum(
 				return nil, err
 			}
 			return tree.NewDTSVector(ret), nil
+		case oidext.T_geometry:
+			ret, err := geo.ParseGeometryFromEWKB(b)
+			if err != nil {
+				return nil, err
+			}
+			return tree.NewDGeometry(ret), nil
+		case oidext.T_geography:
+			ret, err := geo.ParseGeographyFromEWKB(b)
+			if err != nil {
+				return nil, err
+			}
+			return tree.NewDGeography(ret), nil
 		default:
 			if typ.Family() == types.ArrayFamily {
 				return decodeBinaryArray(ctx, evalCtx, typ.ArrayContents(), b, code)

diff --git a/pkg/sql/pgwire/testdata/encodings.json b/pkg/sql/pgwire/testdata/encodings.json
@@ -1098,6 +1098,34 @@
 		"TextAsBinary": [53, 101, 45, 51, 50, 52],
 		"Binary": [0, 0, 0, 0, 0, 0, 0, 1]
 	},
+	{
+		"SQL": "'SRID=4326;POINT EMPTY'::geography",
+		"Oid": 90002,
+		"Text": "0101000020E6100000000000000000F87F000000000000F87F",
+		"TextAsBinary": [48, 49, 48, 49, 48, 48, 48, 48, 50, 48, 69, 54, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 56, 55, 70, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 56, 55, 70],
+		"Binary": [1, 1, 0, 0, 32, 230, 16, 0, 0, 0, 0, 0, 0, 0, 0, 248, 127, 0, 0, 0, 0, 0, 0, 248, 127]
+	},
+	{
+		"SQL": "'GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(1 1, 2 2))'::geography",
+		"Oid": 90002,
+		"Text": "0107000020E610000002000000010100000000000000000000000000000000000000010200000002000000000000000000F03F000000000000F03F00000000000000400000000000000040",
+		"TextAsBinary": [48, 49, 48, 55, 48, 48, 48, 48, 50, 48, 69, 54, 49, 48, 48, 48, 48, 48, 48, 50, 48, 48, 48, 48, 48, 48, 48, 49, 48, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 49, 48, 50, 48, 48, 48, 48, 48, 48, 48, 50, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 48, 51, 70, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 48, 51, 70, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 52, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 52, 48],
+		"Binary": [1, 7, 0, 0, 32, 230, 16, 0, 0, 2, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 240, 63, 0, 0, 0, 0, 0, 0, 240, 63, 0, 0, 0, 0, 0, 0, 0, 64, 0, 0, 0, 0, 0, 0, 0, 64]
+	},
+	{
+		"SQL": "'SRID=4326;POINT EMPTY'::geometry",
+		"Oid": 90000,
+		"Text": "0101000020E6100000000000000000F87F000000000000F87F",
+		"TextAsBinary": [48, 49, 48, 49, 48, 48, 48, 48, 50, 48, 69, 54, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 56, 55, 70, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 56, 55, 70],
+		"Binary": [1, 1, 0, 0, 32, 230, 16, 0, 0, 0, 0, 0, 0, 0, 0, 248, 127, 0, 0, 0, 0, 0, 0, 248, 127]
+	},
+	{
+		"SQL": "'GEOMETRYCOLLECTION(POINT(0 0), LINESTRING(1 1, 2 2))'::geometry",
+		"Oid": 90000,
+		"Text": "010700000002000000010100000000000000000000000000000000000000010200000002000000000000000000F03F000000000000F03F00000000000000400000000000000040",
+		"TextAsBinary": [48, 49, 48, 55, 48, 48, 48, 48, 48, 48, 48, 50, 48, 48, 48, 48, 48, 48, 48, 49, 48, 49, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 49, 48, 50, 48, 48, 48, 48, 48, 48, 48, 50, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 48, 51, 70, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 70, 48, 51, 70, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 52, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 52, 48],
+		"Binary": [1, 7, 0, 0, 0, 2, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 240, 63, 0, 0, 0, 0, 0, 0, 240, 63, 0, 0, 0, 0, 0, 0, 0, 64, 0, 0, 0, 0, 0, 0, 0, 64]
+	},
 	{
 		"SQL": "'0.0.0.0'::inet",
 		"Oid": 869,

diff --git a/pkg/sql/pgwire/testdata/pgtest/spatial b/pkg/sql/pgwire/testdata/pgtest/spatial
@@ -0,0 +1,31 @@
+send
+Parse {"Query": "SELECT $1::GEOMETRY"}
+Bind {"ParameterFormatCodes": [1], "Parameters": [{"binary":"0101000020E6100000000000000000F03F000000000000F03F"}]}
+Execute
+Sync
+----
+
+until
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"DataRow","Values":[{"text":"0101000020E6100000000000000000F03F000000000000F03F"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+send
+Parse {"Query": "SELECT $1::GEOGRAPHY"}
+Bind {"ParameterFormatCodes": [1], "Parameters": [{"binary":"0101000020E6100000000000000000F03F000000000000F03F"}]}
+Execute
+Sync
+----
+
+until
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"DataRow","Values":[{"text":"0101000020E6100000000000000000F03F000000000000F03F"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}