Merge #148778 #148784

148778: geomfn: adjust two tests to work on s390x r=yuzefovich a=yuzefovich

`angleFromCoords` function uses `atan` to compute some geometry functions. s390x system has architecture support for that trigonometric function which can produce slightly different result than when computing via math operations, which resulted in a false positive failure in a couple of tests. This commit teaches those tests to use our custom float comparison function that only checks 15 significant decimal digits.

Fixes: #145985.

Release note: None

148784: logictest: adjust a few of geo tests for s390x r=yuzefovich a=yuzefovich

Similar to other patches, we need to do some rounding for a couple of geo logic tests. This is the case since s390x has architecture support for trigonometric functions which are used when computing `ST_Segmentize` and `ST_Rotate` builtins. Note that the "rounding" is performed via regexp replacement on top of the textual result.

Relatedly, we have a couple of tests that print inverted index keys, and for some shapes / geographies we use s2 library which also uses trigonometric functions. Thus, this commit adjusts those tests to account for the minor differences. This behavior suggests another limitation for heterogeneous clusters - we cannot guarantee to correctly read inverted geo indexes when the SQL pod and the KV / storage node run on different architectures. I don't think there is any concern for "pure" s390x clusters since they will behave consistently within the cluster.

This limitation is filed as #148783.

Fixes: #146001.
Fixes: #146007.
Fixes: #146109.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/geo/geomfn/BUILD.bazel

@@ -111,6 +111,7 @@ go_test(
         "//pkg/geo/geopb",
         "//pkg/geo/geos",
         "//pkg/geo/geotest",
+        "//pkg/testutils/floatcmp",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",

pkg/geo/geomfn/angle_test.go

@@ -8,13 +8,32 @@ package geomfn
 import (
 	"fmt"
 	"math"
+	"runtime"
+	"strconv"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/geo"
 	"github.com/cockroachdb/cockroach/pkg/geo/geopb"
+	"github.com/cockroachdb/cockroach/pkg/testutils/floatcmp"
 	"github.com/stretchr/testify/require"
 )
 
+func assertFloatsMatch(t *testing.T, expected, actual float64) {
+	if runtime.GOARCH != "s390x" {
+		require.Equal(t, expected, actual)
+		return
+	}
+	// s390x system has architecture support for 'atan' function (which is used
+	// in 'angleFromCoords') and produces slightly different result than math
+	// operations we do to get the expected number. To go around this, we use
+	// the custom float comparison that checks 15 significant decimal digits.
+	exp := strconv.FormatFloat(expected, 'f', -1, 64)
+	act := strconv.FormatFloat(actual, 'f', -1, 64)
+	matched, err := floatcmp.FloatsMatch(exp, act)
+	require.NoError(t, err)
+	require.True(t, matched)
+}
+
 func TestAngle(t *testing.T) {
 	pf := func(f float64) *float64 {
 		return &f
@@ -58,7 +77,7 @@ func TestAngle(t *testing.T) {
 			angle, err := Angle(g1, g2, g3, g4)
 			require.NoError(t, err)
 			if tc.expected != nil && angle != nil {
-				require.Equal(t, *tc.expected, *angle)
+				assertFloatsMatch(t, *tc.expected, *angle)
 			} else {
 				require.Equal(t, tc.expected, angle)
 			}
@@ -134,7 +153,7 @@ func TestAngleLineString(t *testing.T) {
 			angle, err := AngleLineString(g1, g2)
 			require.NoError(t, err)
 			if tc.expected != nil && angle != nil {
-				require.Equal(t, *tc.expected, *angle)
+				assertFloatsMatch(t, *tc.expected, *angle)
 			} else {
 				require.Equal(t, tc.expected, angle)
 			}

pkg/sql/logictest/testdata/logic_test/distsql_stats

@@ -1672,13 +1672,29 @@ let $hist_id_1
 SELECT histogram_id FROM [SHOW STATISTICS FOR TABLE geo_table]
 WHERE statistics_name = 's' AND column_names = '{geog}'
 
+# s390x produces slightly different inverted index key for the linestring (among
+# other shapes) because it has architecture support for trigonometric functions
+# (used by s2 library), so we have different expectations for s390x vs other
+# systems. s390x is the only big endian system we support right now, so we abuse
+# the corresponding option a bit.
+skipif bigendian
+
 query TIRI colnames,nosort
 SHOW HISTOGRAM $hist_id_1
 ----
 upper_bound                                                                                 range_rows  distinct_range_rows  equal_rows
 '\x42fd1000000000000000000000000000000000bcc00000000000003ffbecde5da115a83ff661bdc396bcdc'  0           0                    1
 '\x42fd5000000000000000000000000000000000bcc00000000000003ffbecde5da115a83ff661bdc396bcdc'  0           0                    1
 
+skipif littleendian
+
+query TIRI colnames,nosort
+SHOW HISTOGRAM $hist_id_1
+----
+upper_bound                                                                                 range_rows  distinct_range_rows  equal_rows
+'\x42fd1000000000000000000000000000000000bcc00000000000003ffbecde5da115a93ff661bdc396bcdc'  0           0                    1
+'\x42fd5000000000000000000000000000000000bcc00000000000003ffbecde5da115a93ff661bdc396bcdc'  0           0                    1
+
 statement ok
 DROP INDEX geo_table@geog_idx_1;
 
@@ -1690,6 +1706,8 @@ let $hist_id_1
 SELECT histogram_id FROM [SHOW STATISTICS FOR TABLE geo_table]
 WHERE statistics_name = 's' AND column_names = '{geog}'
 
+skipif bigendian
+
 query TIRI colnames,nosort
 SHOW HISTOGRAM $hist_id_1
 ----
@@ -1699,6 +1717,17 @@ upper_bound
 '\x42fd4700000000000000000000000000000000bcc00000000000003ffbecde5da115a83ff661bdc396bcdc'  0           0                    1
 '\x42fd5ad4000000000000000000000000000000bcc00000000000003ffbecde5da115a83ff661bdc396bcdc'  0           0                    1
 
+skipif littleendian
+
+query TIRI colnames,nosort
+SHOW HISTOGRAM $hist_id_1
+----
+upper_bound                                                                                 range_rows  distinct_range_rows  equal_rows
+'\x42fd1000000000000000000000000000000000bcc00000000000003ffbecde5da115a93ff661bdc396bcdc'  0           0                    1
+'\x42fd4500000000000000000000000000000000bcc00000000000003ffbecde5da115a93ff661bdc396bcdc'  0           0                    1
+'\x42fd4700000000000000000000000000000000bcc00000000000003ffbecde5da115a93ff661bdc396bcdc'  0           0                    1
+'\x42fd5ad4000000000000000000000000000000bcc00000000000003ffbecde5da115a93ff661bdc396bcdc'  0           0                    1
+
 # Stats for multi-column inverted indexes.
 statement ok
 CREATE TABLE multi_col (

pkg/sql/logictest/testdata/logic_test/geospatial

@@ -4702,11 +4702,15 @@ Square overlapping left and right square  Square (right)
 Square overlapping left and right square  Square overlapping left and right square  true   true   true
 
 # ST_Segmentize
+#
+# ST_Segmentize uses some trigonometric functions implicitly (via s2 library),
+# which have architecture suppport on some systems like s390x, which can produce
+# slightly different results, so we perform "rounding" via regexp replacement.
 query TTT
 SELECT
   dsc,
   ST_AsText(ST_Segmentize(geog, 100000)),
-  ST_AsText(ST_Segmentize(geog, 50000))
+  regexp_replace(ST_AsText(ST_Segmentize(geog, 50000)), '1.000028552944326', '1.000028552944327', 'g')
 FROM geog_operators_test
 ORDER BY dsc
 ----
@@ -4719,7 +4723,7 @@ NULL                                      NULL
 Nested Geometry Collection                GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0)))                                          GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POINT (0 0)))
 Point middle of Left Square               POINT (-0.5 0.5)                                                                               POINT (-0.5 0.5)
 Point middle of Right Square              POINT (0.5 0.5)                                                                                POINT (0.5 0.5)
-Square (left)                             POLYGON ((-1 0, -0.5 0, 0 0, 0 0.5, 0 1, -0.5 1.000038070652873, -1 1, -1 0.5, -1 0))          POLYGON ((-1 0, -0.75 0, -0.5 0, -0.25 0, 0 0, 0 0.25, 0 0.5, 0 0.75, 0 1, -0.249999998550193 1.000028552944327, -0.5 1.000038070652873, -0.750000001449807 1.000028552944326, -1 1, -1 0.75, -1 0.5, -1 0.25, -1 0))
+Square (left)                             POLYGON ((-1 0, -0.5 0, 0 0, 0 0.5, 0 1, -0.5 1.000038070652873, -1 1, -1 0.5, -1 0))          POLYGON ((-1 0, -0.75 0, -0.5 0, -0.25 0, 0 0, 0 0.25, 0 0.5, 0 0.75, 0 1, -0.249999998550193 1.000028552944327, -0.5 1.000038070652873, -0.750000001449807 1.000028552944327, -1 1, -1 0.75, -1 0.5, -1 0.25, -1 0))
 Square (right)                            POLYGON ((0 0, 0.5 0, 1 0, 1 0.5, 1 1, 0.5 1.000038070652873, 0 1, 0 0.5, 0 0))                POLYGON ((0 0, 0.25 0, 0.5 0, 0.75 0, 1 0, 1 0.25, 1 0.5, 1 0.75, 1 1, 0.750000001449807 1.000028552944327, 0.5 1.000038070652873, 0.249999998550193 1.000028552944327, 0 1, 0 0.75, 0 0.5, 0 0.25, 0 0))
 Square overlapping left and right square  POLYGON ((-0.1 0, 0.45 0, 1 0, 1 0.5, 1 1, 0.45 1.000046065796834, -0.1 1, -0.1 0.5, -0.1 0))  POLYGON ((-0.1 0, 0.175 0, 0.45 0, 0.725 0, 1 0, 1 0.25, 1 0.5, 1 0.75, 1 1, 0.725000001929716 1.000034549281259, 0.45 1.000046065796834, 0.174999998070284 1.000034549281259, -0.1 1, -0.1 0.75, -0.1 0.5, -0.1 0.25, -0.1 0))
 
@@ -5469,11 +5473,14 @@ POLYGON ((-0.1 0, 1 0, 1 1, -0.1 1, -0.1 0))           POLYGON ((0 -0.1, 0 1, 1
 POLYGON ((-1 0, 0 0, 0 1, -1 1, -1 0))                 POLYGON ((0 -1, 0 0, 1 0, 1 -1, 0 -1))
 POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))                    POLYGON ((0 0, 0 1, 1 1, 1 0, 0 0))
 
+# ST_Rotate uses some trigonometric functions internally, which have
+# architecture suppport on some systems like s390x, which can produce slightly
+# different results, so we perform "rounding" via regexp replacement.
 query TTT
 SELECT
   ST_AsText(a.geom) d,
   ST_AsText(ST_Rotate(a.geom, pi())),
-  ST_AsText(ST_Rotate(a.geom, pi()/4))
+  regexp_replace(ST_AsText(ST_Rotate(a.geom, pi()/4)), 'POINT \(0.000000000000001 7.071067811865476\)', 'POINT (0 7.071067811865476)', 'g')
 FROM geom_operators_test a
 ORDER BY d ASC
 ----

pkg/sql/opt/exec/execbuilder/testdata/geospatial

@@ -31,40 +31,75 @@ CPut /Table/106/1/1/0 -> /TUPLE/
 CPut /Table/106/1/2/0 -> /TUPLE/
 
 query T kvtrace
-INSERT INTO c VALUES
-  (1, 'POINT(1.0 1.0)', 'POINT(2.0 2.0)'),
-  (2, 'LINESTRING(1.0 1.0, 2.0 2.0)', 'POINT(1.0 1.0)')
+INSERT INTO c VALUES (1, 'POINT(1.0 1.0)', 'POINT(2.0 2.0)')
 ----
 Scan /Table/20/1/10{7-8}
 CPut /Table/107/1/1/0 -> /TUPLE/
 Put /Table/107/2/"B\xfd\x10\x01D\x15@\x80K\xd5\x01?\x91\xdfF\xa2R\x9d9?\x91\xdfF\xa2R\x9d9\x89\x88" -> /BYTES/
 Put /Table/107/3/"B\xfd\x10\x00\x00\x00\x00\x00\x00\x01\x01@\x00\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00\x89\x88" -> /BYTES/
+
+# s390x produces slightly different inverted index key for the linestring (among
+# other shapes) because it has architecture support for trigonometric functions
+# (used by s2 library), so we have different expectations for s390x vs other
+# systems. s390x is the only big endian system we support right now, so we abuse
+# the corresponding option a bit.
+skipif bigendian
+
+query T kvtrace
+INSERT INTO c VALUES (2, 'LINESTRING(1.0 1.0, 2.0 2.0)', 'POINT(1.0 1.0)')
+----
 CPut /Table/107/1/2/0 -> /TUPLE/
 Put /Table/107/2/"B\xfd\x10\x01P\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb9?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8a\x88" -> /BYTES/
 Put /Table/107/2/"B\xfd\x10\x03\xff\xff\xfc\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb9?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8a\x88" -> /BYTES/
 Put /Table/107/2/"B\xfd\x10\x05\x00\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb9?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8a\x88" -> /BYTES/
 Put /Table/107/3/"B\xfd\x10\x00\x00\x00\x00\x00\x00\x01\x01?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00\x8a\x88" -> /BYTES/
 
+skipif littleendian
+
+query T kvtrace
+INSERT INTO c VALUES (2, 'LINESTRING(1.0 1.0, 2.0 2.0)', 'POINT(1.0 1.0)')
+----
+CPut /Table/107/1/2/0 -> /TUPLE/
+Put /Table/107/2/"B\xfd\x10\x01P\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb8?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8a\x88" -> /BYTES/
+Put /Table/107/2/"B\xfd\x10\x03\xff\xff\xfc\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb8?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8a\x88" -> /BYTES/
+Put /Table/107/2/"B\xfd\x10\x05\x00\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb8?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8a\x88" -> /BYTES/
+Put /Table/107/3/"B\xfd\x10\x00\x00\x00\x00\x00\x00\x01\x01?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00\x8a\x88" -> /BYTES/
+
 statement ok
 CREATE INVERTED INDEX geog_idx ON b(geog)
 
 statement ok
 CREATE INVERTED INDEX geom_idx ON b(geom)
 
 query T kvtrace
-INSERT INTO b VALUES
-  (3, 'POINT(1.0 1.0)', 'POINT(2.0 2.0)'),
-  (4, 'LINESTRING(1.0 1.0, 2.0 2.0)', 'POINT(1.0 1.0)')
+INSERT INTO b VALUES (3, 'POINT(1.0 1.0)', 'POINT(2.0 2.0)')
 ----
 CPut /Table/106/1/3/0 -> /TUPLE/
 Put /Table/106/2/"B\xfd\x10\x01D\x15@\x80K\xd5\x01?\x91\xdfF\xa2R\x9d9?\x91\xdfF\xa2R\x9d9\x8b\x88" -> /BYTES/
 Put /Table/106/4/"B\xfd\x10\x00\x00\x00\x00\x00\x00\x01\x01@\x00\x00\x00\x00\x00\x00\x00@\x00\x00\x00\x00\x00\x00\x00\x8b\x88" -> /BYTES/
+
+skipif bigendian
+
+query T kvtrace
+INSERT INTO b VALUES (4, 'LINESTRING(1.0 1.0, 2.0 2.0)', 'POINT(1.0 1.0)')
+----
 CPut /Table/106/1/4/0 -> /TUPLE/
 Put /Table/106/2/"B\xfd\x10\x01P\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb9?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8c\x88" -> /BYTES/
 Put /Table/106/2/"B\xfd\x10\x03\xff\xff\xfc\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb9?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8c\x88" -> /BYTES/
 Put /Table/106/2/"B\xfd\x10\x05\x00\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb9?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8c\x88" -> /BYTES/
 Put /Table/106/4/"B\xfd\x10\x00\x00\x00\x00\x00\x00\x01\x01?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00\x8c\x88" -> /BYTES/
 
+skipif littleendian
+
+query T kvtrace
+INSERT INTO b VALUES (4, 'LINESTRING(1.0 1.0, 2.0 2.0)', 'POINT(1.0 1.0)')
+----
+CPut /Table/106/1/4/0 -> /TUPLE/
+Put /Table/106/2/"B\xfd\x10\x01P\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb8?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8c\x88" -> /BYTES/
+Put /Table/106/2/"B\xfd\x10\x03\xff\xff\xfc\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb8?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8c\x88" -> /BYTES/
+Put /Table/106/2/"B\xfd\x10\x05\x00\x00\x00\x00\x00\x00\x00?\x91\xdfF\xa2R\x9d8?\x91\xdfF\xa2R\x9c\xb8?\xa1\xdfF\xa2R\x9d9?\xa1\xdfF\xa2R\x9dx\x8c\x88" -> /BYTES/
+Put /Table/106/4/"B\xfd\x10\x00\x00\x00\x00\x00\x00\x01\x01?\xf0\x00\x00\x00\x00\x00\x00?\xf0\x00\x00\x00\x00\x00\x00\x8c\x88" -> /BYTES/
+
 statement ok
 CREATE TABLE ltable(
   k int primary key,