From 58d5b238066f3af058a472eb8238c38d8bc9f777 Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Mon, 1 Jun 2020 08:31:59 -0700 Subject: [PATCH 1/3] geo: remove geoproj dependency for now Seems to be breaking Upload Binaries (not sure why) because execgen depends on geo. The dependencies should make it work (not sure why it doesn't), but it doesn't seem important to solve this atm. Release note: None --- Makefile | 2 -- pkg/geo/geo.go | 1 - pkg/geo/geogfn/geogfn.go | 2 -- 3 files changed, 5 deletions(-) diff --git a/Makefile b/Makefile index 7f2f58bc9836..54ff4068331f 100644 --- a/Makefile +++ b/Makefile @@ -1541,8 +1541,6 @@ bin/execgen_out.d: bin/execgen @echo EXECGEN $@; execgen -M $(EXECGEN_TARGETS) >$@.tmp || { rm -f $@.tmp; exit 1; } @mv -f $@.tmp $@ -bin/execgen: $(LIBPROJ) $(CGO_FLAGS_FILES) - # No need to pull all the world in when a user just wants # to know how to invoke `make` or clean up. ifneq ($(build-with-dep-files),) diff --git a/pkg/geo/geo.go b/pkg/geo/geo.go index 8b86b7c9655c..1b2584f2a642 100644 --- a/pkg/geo/geo.go +++ b/pkg/geo/geo.go @@ -15,7 +15,6 @@ import ( "encoding/binary" "github.com/cockroachdb/cockroach/pkg/geo/geopb" - _ "github.com/cockroachdb/cockroach/pkg/geo/geoproj" // blank import to make sure PROJ compiles "github.com/cockroachdb/errors" "github.com/golang/geo/s2" "github.com/twpayne/go-geom" diff --git a/pkg/geo/geogfn/geogfn.go b/pkg/geo/geogfn/geogfn.go index 62dca3d2cc67..064389d3be88 100644 --- a/pkg/geo/geogfn/geogfn.go +++ b/pkg/geo/geogfn/geogfn.go @@ -10,8 +10,6 @@ package geogfn -import _ "github.com/cockroachdb/cockroach/pkg/geo/geoproj" // blank import to make sure PROJ compiles - // UseSphereOrSpheroid indicates whether to use a Sphere or Spheroid // for certain calculations. type UseSphereOrSpheroid bool From ec0d8942ada6fb37951ecc7726d8f2a78c2b4573 Mon Sep 17 00:00:00 2001 From: Rohan Yadav Date: Mon, 1 Jun 2020 11:18:36 -0400 Subject: [PATCH 2/3] types: fix bug where calling String() on a UDT array would panic It seems like I ran into a proto bug, opened https://github.com/gogo/protobuf/issues/693. Release note: None --- pkg/sql/logictest/testdata/logic_test/enums | 11 +++++++++++ pkg/sql/types/types.go | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/pkg/sql/logictest/testdata/logic_test/enums b/pkg/sql/logictest/testdata/logic_test/enums index 3e25ee9838cd..eb66b4afb611 100644 --- a/pkg/sql/logictest/testdata/logic_test/enums +++ b/pkg/sql/logictest/testdata/logic_test/enums @@ -458,3 +458,14 @@ _collision 100082 0 100083 __collision 100083 100082 0 collision 100084 0 100085 ___collision 100085 100084 0 + +# Regression for #49756. +query TT +SELECT + column_name, column_type +FROM + crdb_internal.table_columns +WHERE + descriptor_name = 'enum_array' AND column_name = 'x' +---- +x family:ArrayFamily width:0 precision:0 locale:"" visible_type:0 oid:100064 array_contents: > TypeMeta:<> > time_precision_is_set:false udt_metadata: diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go index 8b7836b6f425..5ac49da7a593 100644 --- a/pkg/sql/types/types.go +++ b/pkg/sql/types/types.go @@ -2190,6 +2190,19 @@ func (t *T) String() string { // DebugString returns a detailed dump of the type protobuf struct, suitable for // debugging scenarios. func (t *T) DebugString() string { + if t.Family() == ArrayFamily && t.ArrayContents().UserDefined() { + // In the case that this type is an array that contains a user defined + // type, the introspection that protobuf performs to generate a string + // representation will panic if the UserDefinedTypeMetadata field of the + // type is populated. It seems to not understand that fields in the element + // T could be not generated by proto, and panics trying to operate on the + // TypeMeta field of the T. To get around this, we just deep copy the T and + // zero out the type metadata to placate proto. See the following issue for + // details: https://github.com/gogo/protobuf/issues/693. + internalTypeCopy := protoutil.Clone(&t.InternalType).(*InternalType) + internalTypeCopy.ArrayContents.TypeMeta = UserDefinedTypeMetadata{} + return internalTypeCopy.String() + } return t.InternalType.String() } From a6f8e4448f6e3754913010f128f6aae1186c892c Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Mon, 1 Jun 2020 09:39:33 -0700 Subject: [PATCH 3/3] c-deps/proj: bump PROJ to compile on more systems CMake is broken for the value of `PTHREAD_MUTEX_RECURSIVE` on certain operating systems. To rectify this, we've patched PROJ at version 4.9.3 at `cc4a178` with a fix. We'll be using the CRDB fork of PROJ for the meantime to fix this for now. Release note: None --- .gitmodules | 2 +- c-deps/proj | 2 +- c-deps/proj-rebuild | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitmodules b/.gitmodules index 0086c3bfdb82..4460ccfcfed1 100644 --- a/.gitmodules +++ b/.gitmodules @@ -34,4 +34,4 @@ url = https://github.com/libgeos/geos.git [submodule "c-deps/proj"] path = c-deps/proj - url = https://github.com/OSGeo/PROJ.git + url = https://github.com/cockroachdb/PROJ.git diff --git a/c-deps/proj b/c-deps/proj index da2d678c387b..9016763fb905 160000 --- a/c-deps/proj +++ b/c-deps/proj @@ -1 +1 @@ -Subproject commit da2d678c387ba76704824360dc52771c29d94693 +Subproject commit 9016763fb90519740ce1be1a1956505ca0710f7d diff --git a/c-deps/proj-rebuild b/c-deps/proj-rebuild index b5a851d4e5e2..14b89c8508ff 100644 --- a/c-deps/proj-rebuild +++ b/c-deps/proj-rebuild @@ -1,4 +1,4 @@ Bump the version below when changing geos configure flags. Search for "BUILD ARTIFACT CACHING" in build/common.mk for rationale. -1 +2