From df4e6eeb358d0e6a440cb7def21639884892cfd6 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Sun, 6 Sep 2026 00:45:12 +0300 Subject: [PATCH] clamped arccosine to [-1,1] to prevent rounding errors in geoDistance --- CHANGELOG.md | 2 + tools/search/filter_test.go | 2 +- tools/search/token_functions.go | 7 ++- tools/search/token_functions_test.go | 80 +++++++++++++++++++--------- 4 files changed, 63 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8087ff9..ed72ea75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Fixed collection index validator to allow expressions with parenthesis in the optional `WHERE` clause. +- Clamped arccosine to [-1,1] in the Harvesine formula for the `geoDistance()` filter function to workaround edge case related to float rounding errors for some coordinates. + - Bumped `golang.org/x/*` dependencies to silence security scanners ([#7829](https://github.com/pocketbase/pocketbase/discussions/7829)). diff --git a/tools/search/filter_test.go b/tools/search/filter_test.go index 6f5d7663..9322bdd5 100644 --- a/tools/search/filter_test.go +++ b/tools/search/filter_test.go @@ -144,7 +144,7 @@ func TestFilterDataBuildExpr(t *testing.T) { "geoDistance function", "geoDistance(1,2,3,4) < 567", false, - "(6371 * acos(cos(radians({:TEST})) * cos(radians({:TEST})) * cos(radians({:TEST}) - radians({:TEST})) + sin(radians({:TEST})) * sin(radians({:TEST})))) < {:TEST}", + "(6371 * acos(min(1, max(-1, cos(radians({:TEST})) * cos(radians({:TEST})) * cos(radians({:TEST}) - radians({:TEST})) + sin(radians({:TEST})) * sin(radians({:TEST})))))) < {:TEST}", }, } diff --git a/tools/search/token_functions.go b/tools/search/token_functions.go index 1fc5449a..03edd5e6 100644 --- a/tools/search/token_functions.go +++ b/tools/search/token_functions.go @@ -49,11 +49,14 @@ var TokenFunctions = map[string]func( return &ResolverResult{ NullFallback: NullFallbackDisabled, - Identifier: `(6371 * acos(` + + // the clamping is to prevent floating point rounding errors for values like + // "1.0002" that could occur for example when comparing identical points + // (see the NULL note for arccosine in https://sqlite.org/lang_mathfunc.html#overview) + Identifier: `(6371 * acos(min(1, max(-1, ` + `cos(radians(` + latA + `)) * cos(radians(` + latB + `)) * ` + `cos(radians(` + lonB + `) - radians(` + lonA + `)) + ` + `sin(radians(` + latA + `)) * sin(radians(` + latB + `))` + - `))`, + `))))`, Params: mergeParams(resolvedArgs[0].Params, resolvedArgs[1].Params, resolvedArgs[2].Params, resolvedArgs[3].Params), }, nil }, diff --git a/tools/search/token_functions_test.go b/tools/search/token_functions_test.go index fa9ed6a4..7221145e 100644 --- a/tools/search/token_functions_test.go +++ b/tools/search/token_functions_test.go @@ -117,7 +117,7 @@ func TestTokenFunctionsGeoDistance(t *testing.T) { baseTokenResolver, &ResolverResult{ NullFallback: NullFallbackDisabled, - Identifier: `(6371 * acos(cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))`, + Identifier: `(6371 * acos(min(1, max(-1, cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))))`, Params: map[string]any{ "lonA": 1, "latA": 2, @@ -138,7 +138,7 @@ func TestTokenFunctionsGeoDistance(t *testing.T) { baseTokenResolver, &ResolverResult{ NullFallback: NullFallbackDisabled, - Identifier: `(6371 * acos(cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))`, + Identifier: `(6371 * acos(min(1, max(-1, cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))))`, Params: map[string]any{ "lonA": "null", "latA": 2, @@ -178,34 +178,64 @@ func TestTokenFunctionsGeoDistanceExec(t *testing.T) { t.Error("Expected geoDistance token function to be registered.") } - result, err := fn( - func(t fexpr.Token) (*ResolverResult, error) { - placeholder := "t" + security.PseudorandomString(5) - return &ResolverResult{Identifier: "{:" + placeholder + "}", Params: map[string]any{placeholder: t.Literal}}, nil + scenarios := []struct { + name string + coords []string // [lonA, latA, lonB, latB] + expected string + }{ + { + "distinct points", + []string{ + "23.23033854945808", + "42.713146090563384", + "23.44920680886216", + "42.7078484153991", + }, + "17.89", + }, + { + "identical points (rounding check with 8 lat)", + []string{"0", "8", "0", "8"}, // 8 is an example latitude where rounding happens to result in > 1.0 + "0.00", + }, + { + "identical points (rounding check with 45 lat)", + []string{"0", "45", "0", "45"}, // 45 is an example latitude where rounding happens to result in > 1.0 + "0.00", }, - fexpr.Token{Literal: "23.23033854945808", Type: fexpr.TokenNumber}, - fexpr.Token{Literal: "42.713146090563384", Type: fexpr.TokenNumber}, - fexpr.Token{Literal: "23.44920680886216", Type: fexpr.TokenNumber}, - fexpr.Token{Literal: "42.7078484153991", Type: fexpr.TokenNumber}, - ) - if err != nil { - t.Fatal(err) } - column := []float64{} - err = testDB.NewQuery("select " + result.Identifier).Bind(result.Params).Column(&column) - if err != nil { - t.Fatal(err) - } + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + result, err := fn( + func(t fexpr.Token) (*ResolverResult, error) { + placeholder := "t" + security.PseudorandomString(5) + return &ResolverResult{Identifier: "{:" + placeholder + "}", Params: map[string]any{placeholder: t.Literal}}, nil + }, + fexpr.Token{Literal: s.coords[0], Type: fexpr.TokenNumber}, + fexpr.Token{Literal: s.coords[1], Type: fexpr.TokenNumber}, + fexpr.Token{Literal: s.coords[2], Type: fexpr.TokenNumber}, + fexpr.Token{Literal: s.coords[3], Type: fexpr.TokenNumber}, + ) + if err != nil { + t.Fatal(err) + } - if len(column) != 1 { - t.Fatalf("Expected exactly 1 column value as result, got %v", column) - } + column := []float64{} + err = testDB.NewQuery("select " + result.Identifier).Bind(result.Params).Column(&column) + if err != nil { + t.Fatal(err) + } - expected := "17.89" - distance := fmt.Sprintf("%.2f", column[0]) - if distance != expected { - t.Fatalf("Expected distance value %s, got %s", expected, distance) + if len(column) != 1 { + t.Fatalf("Expected exactly 1 column value as result, got %v", column) + } + + distance := fmt.Sprintf("%.2f", column[0]) + if distance != s.expected { + t.Fatalf("Expected distance value %s, got %s", s.expected, distance) + } + }) } }