replaced NoCoalesce with NullFallback and updated tests

2026-01-21 21:00:15 -05:00 · 2026-01-15 18:06:15 +02:00
parent b0a5bce4c4
commit bbd7f4e4ae
7 changed files with 85 additions and 55 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,17 +8,19 @@
    For some queries and data sets the above 2 optimizations have shown significant improvements but if you notice a performance degradation after upgrading,
    please open a Q&A discussion with export of your collections structure and the problematic request so that it can be analyzed.

- ⚠️ Replaced the expression interface of `search.ResolverResult.MultiMatchSubQuery` with the concrete struct type `search.MultiMatchSubquery` to avoid excessive type assertions and allow direct mutations of the field.
+- ⚠️ `search.ResolverResult` struct changes _(mostly used internally)_:
+    - Replaced `NoCoalesce` field with the more explicit `NullFallback` _(`NullFallbackDisabled` is the same as `NoCoalesce:true`)_.
+    - Replaced the expression interface of the `MultiMatchSubQuery` field with the concrete struct type `search.MultiMatchSubquery` to avoid excessive type assertions and allow direct mutations of the field.

 - Added [`strftime(format, [timevalue, modifiers...])`](@todo link to docs) date formatting filter and API rules function.
    It operates similarly to the equivalent [SQLite `strftime` builtin function](https://sqlite.org/lang_datefunc.html)
    with the exception that for some operators the result will be coalesced for consistency with the non-nullable behavior of the default PocketBase fields.
    Multi-match expressions are also supported and works the same as if the collection field is referenced, for example:
    ```js
-    // requires any/at-least-one-of multiRel records to have created date matching the formatted string "2026-01"
+    // requires ANY/AT-LEAST-ONE-OF multiRel records to have "created" date matching the formatted string "2026-01"
    strftime('%Y-%m', multiRel.created) ?= "2026-01"

-    // requires ALL multiRel records to have created date matching the formatted string "2026-01"
+    // requires ALL multiRel records to have "created" date matching the formatted string "2026-01"
    strftime('%Y-%m', multiRel.created) = "2026-01"
    ```

--- a/core/record_field_resolver_runner.go
+++ b/core/record_field_resolver_runner.go
@@ -285,8 +285,8 @@ func (r *runner) processRequestBodyChangedModifier(bodyField Field) (*search.Res
 	placeholder := "@changed@" + name + security.PseudorandomString(8)

 	result := &search.ResolverResult{
-		Identifier: placeholder,
-		NoCoalesce: true,
+		Identifier:   placeholder,
+		NullFallback: search.NullFallbackDisabled,
 		AfterBuild: func(expr dbx.Expression) dbx.Expression {
 			return &replaceWithExpression{
 				placeholder: placeholder,
@@ -477,8 +477,8 @@ func (r *runner) processActiveProps() (*search.ResolverResult, error) {
 			jsonPathStr := jsonPath.String()

 			result := &search.ResolverResult{
-				NoCoalesce: true,
-				Identifier: dbutils.JSONExtract(r.activeTableAlias+"."+inflector.Columnify(prop), jsonPathStr),
+				NullFallback: search.NullFallbackDisabled,
+				Identifier:   dbutils.JSONExtract(r.activeTableAlias+"."+inflector.Columnify(prop), jsonPathStr),
 			}

 			if r.withMultiMatch {
@@ -834,7 +834,7 @@ func (r *runner) finalizeActivePropsProcessing(collection *Collection, prop stri
 	// stored as json work correctly when compared to their SQL equivalent
 	// (https://github.com/pocketbase/pocketbase/issues/4068)
 	if field.Type() == FieldTypeJSON {
-		result.NoCoalesce = true
+		result.NullFallback = search.NullFallbackDisabled
 		result.Identifier = dbutils.JSONExtract(r.activeTableAlias+"."+cleanFieldName, "")
 		if r.withMultiMatch {
 			r.multiMatch.ValueIdentifier = dbutils.JSONExtract(r.multiMatchActiveTableAlias+"."+cleanFieldName, "")
--- a/core/record_field_resolver_test.go
+++ b/core/record_field_resolver_test.go
@@ -645,11 +645,11 @@ func TestRecordFieldResolverUpdateQuery(t *testing.T) {
 			"SELECT `view1`.* FROM `view1` WHERE (([[view1.point]] = '' OR [[view1.point]] IS NULL) OR (CASE WHEN json_valid([[view1.point]]) THEN JSON_EXTRACT([[view1.point]], '$.lat') ELSE JSON_EXTRACT(json_object('pb', [[view1.point]]), '$.pb.lat') END) > {:TEST} OR (CASE WHEN json_valid([[view1.point]]) THEN JSON_EXTRACT([[view1.point]], '$.lon') ELSE JSON_EXTRACT(json_object('pb', [[view1.point]]), '$.pb.lon') END) < {:TEST} OR (CASE WHEN json_valid([[view1.point]]) THEN JSON_EXTRACT([[view1.point]], '$.something') ELSE JSON_EXTRACT(json_object('pb', [[view1.point]]), '$.pb.something') END) > {:TEST})",
 		},
 		{
-			"strftime with fixed string as time-value",
+			"strftime with fixed string as time-value against known empty value (null normalizations)",
 			"demo5",
-			"strftime('%Y-%m', '2026-01-01') = true",
+			"strftime('%Y-%m', '2026-01-01') = ''",
 			false,
-			"SELECT `demo5`.* FROM `demo5` WHERE strftime({:TEST},{:TEST}) = 1",
+			"SELECT `demo5`.* FROM `demo5` WHERE ((strftime({:TEST},{:TEST}) = '' OR strftime({:TEST},{:TEST}) IS NULL))",
 		},
 		{
 			"strftime without multi-match",
--- a/tools/search/filter.go
+++ b/tools/search/filter.go
@@ -218,7 +218,7 @@ func buildResolversExpr(
 			expr = dbx.Enclose(dbx.And(expr, mm))
 		} else if left.MultiMatchSubQuery != nil {
 			mm := &manyVsOneExpr{
-				noCoalesce:   left.NoCoalesce,
+				nullFallback: left.NullFallback,
 				subQuery:     left.MultiMatchSubQuery,
 				op:           op,
 				otherOperand: right,
@@ -227,7 +227,7 @@ func buildResolversExpr(
 			expr = dbx.Enclose(dbx.And(expr, mm))
 		} else if right.MultiMatchSubQuery != nil {
 			mm := &manyVsOneExpr{
-				noCoalesce:   right.NoCoalesce,
+				nullFallback: right.NullFallback,
 				subQuery:     right.MultiMatchSubQuery,
 				op:           op,
 				otherOperand: left,
@@ -326,9 +326,6 @@ func resolveToken(token fexpr.Token, fieldResolver FieldResolver) (*ResolverResu
 // `COALESCE(a, "") = ""` since the direct match can be accomplished
 // with a seek while the COALESCE will induce a table scan.
 func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
-	isLeftEmpty := isEmptyIdentifier(left) || (len(left.Params) == 1 && hasEmptyParamValue(left))
-	isRightEmpty := isEmptyIdentifier(right) || (len(right.Params) == 1 && hasEmptyParamValue(right))
-
 	equalOp := "="
 	nullEqualOp := "IS"
 	concatOp := "OR"
@@ -343,16 +340,23 @@ func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
 		nullExpr = "IS NOT NULL"
 	}

-	// no coalesce (eg. compare to a json field)
+	// no coalesce fallback (eg. compare to a json field)
 	// a IS b
 	// a IS NOT b
-	if left.NoCoalesce || right.NoCoalesce {
+	if left.NullFallback == NullFallbackDisabled ||
+		right.NullFallback == NullFallbackDisabled {
 		return dbx.NewExp(
 			fmt.Sprintf("%s %s %s", left.Identifier, nullEqualOp, right.Identifier),
 			mergeParams(left.Params, right.Params),
 		)
 	}

+	isLeftEmpty := isEmptyIdentifier(left) ||
+		(left.NullFallback == NullFallbackAuto && len(left.Params) == 1 && hasEmptyParamValue(left))
+
+	isRightEmpty := isEmptyIdentifier(right) ||
+		(right.NullFallback == NullFallbackAuto && len(right.Params) == 1 && hasEmptyParamValue(right))
+
 	// both operands are empty
 	if isLeftEmpty && isRightEmpty {
 		return dbx.NewExp(fmt.Sprintf("'' %s ''", equalOp), mergeParams(left.Params, right.Params))
@@ -421,6 +425,10 @@ func hasEmptyParamValue(result *ResolverResult) bool {
 }

 func isKnownNonEmptyIdentifier(result *ResolverResult) bool {
+	if result.NullFallback == NullFallbackEnforced {
+		return false
+	}
+
 	switch strings.ToLower(result.Identifier) {
 	case "1", "0", "false", `true`:
 		return true
@@ -631,13 +639,13 @@ func (e *manyVsManyExpr) Build(db *dbx.DB, params dbx.Params) string {

 	whereExpr, buildErr := buildResolversExpr(
 		&ResolverResult{
-			NoCoalesce: e.left.NoCoalesce,
-			Identifier: "[[" + lAlias + ".multiMatchValue]]",
+			NullFallback: e.left.NullFallback,
+			Identifier:   "[[" + lAlias + ".multiMatchValue]]",
 		},
 		e.op,
 		&ResolverResult{
-			NoCoalesce: e.right.NoCoalesce,
-			Identifier: "[[" + rAlias + ".multiMatchValue]]",
+			NullFallback: e.right.NullFallback,
+			Identifier:   "[[" + rAlias + ".multiMatchValue]]",
 			// note: the AfterBuild needs to be handled only once and it
 			// doesn't matter whether it is applied on the left or right subquery operand
 			AfterBuild: dbx.Not, // inverse for the not-exist expression
@@ -672,7 +680,7 @@ type manyVsOneExpr struct {
 	subQuery     dbx.Expression
 	op           fexpr.SignOp
 	inverse      bool
-	noCoalesce   bool
+	nullFallback NullFallbackPreference
 }

 // Build converts the expression into a SQL fragment.
@@ -686,9 +694,9 @@ func (e *manyVsOneExpr) Build(db *dbx.DB, params dbx.Params) string {
 	alias := "__sm" + security.PseudorandomString(8)

 	r1 := &ResolverResult{
-		NoCoalesce: e.noCoalesce,
-		Identifier: "[[" + alias + ".multiMatchValue]]",
-		AfterBuild: dbx.Not, // inverse for the not-exist expression
+		NullFallback: e.nullFallback,
+		Identifier:   "[[" + alias + ".multiMatchValue]]",
+		AfterBuild:   dbx.Not, // inverse for the not-exist expression
 	}

 	r2 := &ResolverResult{
--- a/tools/search/simple_field_resolver.go
+++ b/tools/search/simple_field_resolver.go
@@ -10,15 +10,26 @@ import (
 	"github.com/pocketbase/pocketbase/tools/list"
 )

+type NullFallbackPreference int
+
+const (
+	NullFallbackAuto     NullFallbackPreference = 0
+	NullFallbackDisabled NullFallbackPreference = 1
+	NullFallbackEnforced NullFallbackPreference = 2
+)
+
 // ResolverResult defines a single FieldResolver.Resolve() successfully parsed result.
 type ResolverResult struct {
 	// Identifier is the plain SQL identifier/column that will be used
 	// in the final db expression as left or right operand.
 	Identifier string

-	// NoCoalesce instructs to not use COALESCE or NULL fallbacks
-	// when building the identifier expression.
-	NoCoalesce bool
+	// NullFallback specify the preference for how NULL or empty values
+	// should be resolved (default to "auto").
+	//
+	// Set to NullFallbackDisabled to prevent any COALESCE or NULL fallbacks.
+	// Set to NullFallbackEnforced to prefer COALESCE or NULL fallbacks when needed.
+	NullFallback NullFallbackPreference

 	// Params is a map with db placeholder->value pairs that will be added
 	// to the query when building both resolved operands/sides in a single expression.
@@ -103,7 +114,7 @@ func (r *SimpleFieldResolver) Resolve(field string) (*ResolverResult, error) {
 	}

 	return &ResolverResult{
-		NoCoalesce: true,
+		NullFallback: NullFallbackDisabled,
 		Identifier: fmt.Sprintf(
 			"JSON_EXTRACT([[%s]], '%s')",
 			inflector.Columnify(parts[0]),
--- a/tools/search/token_functions.go
+++ b/tools/search/token_functions.go
@@ -48,7 +48,7 @@ var TokenFunctions = map[string]func(
 		latB := resolvedArgs[3].Identifier

 		return &ResolverResult{
-			NoCoalesce: true,
+			NullFallback: NullFallbackDisabled,
 			Identifier: `(6371 * acos(` +
 				`cos(radians(` + latA + `)) * cos(radians(` + latB + `)) * ` +
 				`cos(radians(` + lonB + `) - radians(` + lonA + `)) + ` +
@@ -61,12 +61,14 @@ var TokenFunctions = map[string]func(
 	// strftime(format, [timeValue, modifier1, modifier2, ...]) returns
 	// a date string formatted according to the specified format argument.
 	//
-	// It is similar to the builtin SQLite strftime function (https://sqlite.org/lang_datefunc.html).
+	// It is similar to the builtin SQLite strftime function (https://sqlite.org/lang_datefunc.html)
+	// with the main difference that NULL results will be normalized for
+	// consistency with the non-nullable PocketBase "text" and "date" fields.
 	//
-	// It accepts 1, 2 or 3+ arguments.
+	// The function accepts 1, 2 or 3+ arguments.
 	//
 	// (1) The first (format) argument must be always a formatting string
-	// with valid substitutions listed in https://sqlite.org/lang_datefunc.html.
+	// with valid substitutions as listed in https://sqlite.org/lang_datefunc.html.
 	//
 	// (2) The second (time-value) argument is optional and must be either a date string, number or collection field identifier
 	// that matches one of the formats listed in https://sqlite.org/lang_datefunc.html#time_values.
@@ -74,9 +76,6 @@ var TokenFunctions = map[string]func(
 	// (3+) The remaining (modifiers) optional arguments are expected to be
 	// string literals matching the listed modifiers in https://sqlite.org/lang_datefunc.html#modifiers.
 	//
-	// Note that an invalid format, time-value, or modifier could result in COALESCE(strftime(...), null)
-	// for consistency with the non-null nature of the default PocketBase fields.
-	//
 	// A multi-match constraint will be also applied in case the time-value
 	// is an identifier as a result of a multi-value relation field.
 	"strftime": func(argTokenResolverFunc func(fexpr.Token) (*ResolverResult, error), args ...fexpr.Token) (*ResolverResult, error) {
@@ -104,6 +103,7 @@ var TokenFunctions = map[string]func(

 		// no further arguments
 		if totalArgs == 1 {
+			formatArgResult.NullFallback = NullFallbackEnforced
 			formatArgResult.Identifier = "strftime(" + formatArgResult.Identifier + ")"
 			return formatArgResult, nil
 		}
@@ -138,7 +138,10 @@ var TokenFunctions = map[string]func(

 		// generating new ResolverResult
 		// -----------------------------------------------------------
-		result := &ResolverResult{Params: dbx.Params{}}
+		result := &ResolverResult{
+			NullFallback: NullFallbackEnforced,
+			Params:       dbx.Params{},
+		}

 		identifiers := make([]string, 0, totalArgs)

--- a/tools/search/token_functions_test.go
+++ b/tools/search/token_functions_test.go
@@ -116,8 +116,8 @@ func TestTokenFunctionsGeoDistance(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				NoCoalesce: true,
-				Identifier: `(6371 * acos(cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))`,
+				NullFallback: NullFallbackDisabled,
+				Identifier:   `(6371 * acos(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,
@@ -137,8 +137,8 @@ func TestTokenFunctionsGeoDistance(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				NoCoalesce: true,
-				Identifier: `(6371 * acos(cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))`,
+				NullFallback: NullFallbackDisabled,
+				Identifier:   `(6371 * acos(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,
@@ -288,8 +288,9 @@ func TestTokenFunctionsStrftime(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				Identifier: `strftime({:format})`,
-				Params:     map[string]any{"format": "abc"},
+				NullFallback: NullFallbackEnforced,
+				Identifier:   `strftime({:format})`,
+				Params:       map[string]any{"format": "abc"},
 			},
 			false,
 		},
@@ -324,8 +325,9 @@ func TestTokenFunctionsStrftime(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				Identifier: `strftime({:format},{:time})`,
-				Params:     map[string]any{"format": "1", "time": "2"},
+				NullFallback: NullFallbackEnforced,
+				Identifier:   `strftime({:format},{:time})`,
+				Params:       map[string]any{"format": "1", "time": "2"},
 			},
 			false,
 		},
@@ -337,8 +339,9 @@ func TestTokenFunctionsStrftime(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				Identifier: `strftime({:format},{:time})`,
-				Params:     map[string]any{"format": "1", "time": "2"},
+				NullFallback: NullFallbackEnforced,
+				Identifier:   `strftime({:format},{:time})`,
+				Params:       map[string]any{"format": "1", "time": "2"},
 			},
 			false,
 		},
@@ -350,8 +353,9 @@ func TestTokenFunctionsStrftime(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				Identifier: `strftime({:format},{:time})`,
-				Params:     map[string]any{"format": "1", "time": "2"},
+				NullFallback: NullFallbackEnforced,
+				Identifier:   `strftime({:format},{:time})`,
+				Params:       map[string]any{"format": "1", "time": "2"},
 			},
 			false,
 		},
@@ -416,8 +420,9 @@ func TestTokenFunctionsStrftime(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				Identifier: `strftime({:format},{:time},{:m1},{:m2})`,
-				Params:     map[string]any{"format": "1", "time": "2", "m1": "3", "m2": "4"},
+				NullFallback: NullFallbackEnforced,
+				Identifier:   `strftime({:format},{:time},{:m1},{:m2})`,
+				Params:       map[string]any{"format": "1", "time": "2", "m1": "3", "m2": "4"},
 			},
 			false,
 		},
@@ -440,7 +445,8 @@ func TestTokenFunctionsStrftime(t *testing.T) {
 			},
 			baseTokenResolver,
 			&ResolverResult{
-				Identifier: `strftime({:format},{:time},{:m1},{:m2},{:m3},{:m4},{:m5},{:m6},{:m7},{:m8})`,
+				NullFallback: NullFallbackEnforced,
+				Identifier:   `strftime({:format},{:time},{:m1},{:m2},{:m3},{:m4},{:m5},{:m6},{:m7},{:m8})`,
 				Params: map[string]any{
 					"format": "1",
 					"time":   "2",
@@ -597,8 +603,8 @@ func testCompareResults(t *testing.T, a, b *ResolverResult) {
 		t.Fatalf("Expected bMultiMatchSubQuery and bMultiMatchSubQuery to be the same, got\n%s\nvs\n%s", aMultiMatchSubQuery, bMultiMatchSubQuery)
 	}

-	if a.NoCoalesce != b.NoCoalesce {
-		t.Fatalf("Expected NoCoalesce to match, got %v vs %v", a.NoCoalesce, b.NoCoalesce)
+	if a.NullFallback != b.NullFallback {
+		t.Fatalf("Expected NullFallback to match, got %v vs %v", a.NullFallback, b.NullFallback)
 	}

 	if len(a.Params) != len(b.Params) {