Skip to content

Commit

Permalink
[pocketbase#4068] fixed the json field query comparisons to work co…
Browse files Browse the repository at this point in the history
…rrectly with plain JSON values
  • Loading branch information
ganigeorgiev committed Jan 3, 2024
1 parent 8f625da commit 4f24922
Show file tree
Hide file tree
Showing 42 changed files with 119 additions and 54 deletions.
4 changes: 4 additions & 0 deletions forms/validators/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
)

func TestUploadedFileSize(t *testing.T) {
t.Parallel()

data, mp, err := tests.MockMultipartData(nil, "test")
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -52,6 +54,8 @@ func TestUploadedFileSize(t *testing.T) {
}

func TestUploadedFileMimeType(t *testing.T) {
t.Parallel()

data, mp, err := tests.MockMultipartData(nil, "test")
if err != nil {
t.Fatal(err)
Expand Down
2 changes: 2 additions & 0 deletions forms/validators/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
)

func TestUniqueId(t *testing.T) {
t.Parallel()

app, _ := tests.NewTestApp()
defer app.Cleanup()

Expand Down
2 changes: 2 additions & 0 deletions forms/validators/record_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ func TestRecordDataValidatorEmptyAndUnknown(t *testing.T) {
}

func TestRecordDataValidatorValidateText(t *testing.T) {
t.Parallel()

app, _ := tests.NewTestApp()
defer app.Cleanup()

Expand Down
2 changes: 2 additions & 0 deletions forms/validators/string_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
)

func TestCompare(t *testing.T) {
t.Parallel()

scenarios := []struct {
valA string
valB string
Expand Down
2 changes: 1 addition & 1 deletion plugins/jsvm/internal/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ declare function readerToString(reader: any, maxBytes?: number): string;
* Example:
*
* ` + "```" + `js
* slee(250) // sleeps for 250ms
* sleep(250) // sleeps for 250ms
* ` + "```" + `
*
* @group PocketBase
Expand Down
12 changes: 12 additions & 0 deletions resolvers/record_field_resolve_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,17 @@ func (r *runner) processActiveProps() (*search.ResolverResult, error) {
result.MultiMatchSubQuery = r.multiMatch
}

// wrap in json_extract to ensure that top-level primitives
// stored as json work correctly when compared to their SQL equivalent
// (https://github.com/pocketbase/pocketbase/issues/4068)
if field.Type == schema.FieldTypeJson {
result.NoCoalesce = true
result.Identifier = "JSON_EXTRACT(" + result.Identifier + ", '$')"
if r.withMultiMatch {
r.multiMatch.valueIdentifier = "JSON_EXTRACT(" + r.multiMatch.valueIdentifier + ", '$')"
}
}

return result, nil
}

Expand Down Expand Up @@ -480,6 +491,7 @@ func (r *runner) processActiveProps() (*search.ResolverResult, error) {
}

result := &search.ResolverResult{
NoCoalesce: true,
Identifier: fmt.Sprintf(
"JSON_EXTRACT([[%s.%s]], '%s')",
r.activeTableAlias,
Expand Down
16 changes: 15 additions & 1 deletion resolvers/record_field_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,21 @@ func TestRecordFieldResolverUpdateQuery(t *testing.T) {
"demo4",
"json_object.a.b = '' && self_rel_many:length != 2 && json_object.a.b > 3 && self_rel_many:length <= 4",
false,
"SELECT `demo4`.* FROM `demo4` WHERE ((JSON_EXTRACT([[demo4.json_object]], '$.a.b') = '' OR JSON_EXTRACT([[demo4.json_object]], '$.a.b') IS NULL) AND json_array_length(CASE WHEN json_valid([[demo4.self_rel_many]]) THEN [[demo4.self_rel_many]] ELSE (CASE WHEN [[demo4.self_rel_many]] = '' OR [[demo4.self_rel_many]] IS NULL THEN json_array() ELSE json_array([[demo4.self_rel_many]]) END) END) IS NOT {:TEST} AND JSON_EXTRACT([[demo4.json_object]], '$.a.b') > {:TEST} AND json_array_length(CASE WHEN json_valid([[demo4.self_rel_many]]) THEN [[demo4.self_rel_many]] ELSE (CASE WHEN [[demo4.self_rel_many]] = '' OR [[demo4.self_rel_many]] IS NULL THEN json_array() ELSE json_array([[demo4.self_rel_many]]) END) END) <= {:TEST})",
"SELECT `demo4`.* FROM `demo4` WHERE (JSON_EXTRACT([[demo4.json_object]], '$.a.b') IS {:TEST} AND json_array_length(CASE WHEN json_valid([[demo4.self_rel_many]]) THEN [[demo4.self_rel_many]] ELSE (CASE WHEN [[demo4.self_rel_many]] = '' OR [[demo4.self_rel_many]] IS NULL THEN json_array() ELSE json_array([[demo4.self_rel_many]]) END) END) IS NOT {:TEST} AND JSON_EXTRACT([[demo4.json_object]], '$.a.b') > {:TEST} AND json_array_length(CASE WHEN json_valid([[demo4.self_rel_many]]) THEN [[demo4.self_rel_many]] ELSE (CASE WHEN [[demo4.self_rel_many]] = '' OR [[demo4.self_rel_many]] IS NULL THEN json_array() ELSE json_array([[demo4.self_rel_many]]) END) END) <= {:TEST})",
},
{
"json field equal normalization checks",
"demo4",
"json_object = '' || json_object != '' || '' = json_object || '' != json_object ||" +
"json_object = null || json_object != null || null = json_object || null != json_object ||" +
"json_object = true || json_object != true || true = json_object || true != json_object ||" +
"json_object = json_object || json_object != json_object ||" +
"json_object = title || title != json_object ||" +
// multimatch expressions
"self_rel_many.json_object = '' || null = self_rel_many.json_object ||" +
"self_rel_many.json_object = self_rel_many.json_object",
false,
"SELECT DISTINCT `demo4`.* FROM `demo4` LEFT JOIN json_each(CASE WHEN json_valid([[demo4.self_rel_many]]) THEN [[demo4.self_rel_many]] ELSE json_array([[demo4.self_rel_many]]) END) `demo4_self_rel_many_je` LEFT JOIN `demo4` `demo4_self_rel_many` ON [[demo4_self_rel_many.id]] = [[demo4_self_rel_many_je.value]] WHERE (JSON_EXTRACT([[demo4.json_object]], '$') IS {:TEST} OR JSON_EXTRACT([[demo4.json_object]], '$') IS NOT {:TEST} OR {:TEST} IS JSON_EXTRACT([[demo4.json_object]], '$') OR {:TEST} IS NOT JSON_EXTRACT([[demo4.json_object]], '$') OR JSON_EXTRACT([[demo4.json_object]], '$') IS NULL OR JSON_EXTRACT([[demo4.json_object]], '$') IS NOT NULL OR NULL IS JSON_EXTRACT([[demo4.json_object]], '$') OR NULL IS NOT JSON_EXTRACT([[demo4.json_object]], '$') OR JSON_EXTRACT([[demo4.json_object]], '$') IS 1 OR JSON_EXTRACT([[demo4.json_object]], '$') IS NOT 1 OR 1 IS JSON_EXTRACT([[demo4.json_object]], '$') OR 1 IS NOT JSON_EXTRACT([[demo4.json_object]], '$') OR JSON_EXTRACT([[demo4.json_object]], '$') IS JSON_EXTRACT([[demo4.json_object]], '$') OR JSON_EXTRACT([[demo4.json_object]], '$') IS NOT JSON_EXTRACT([[demo4.json_object]], '$') OR JSON_EXTRACT([[demo4.json_object]], '$') IS [[demo4.title]] OR [[demo4.title]] IS NOT JSON_EXTRACT([[demo4.json_object]], '$') OR ((JSON_EXTRACT([[demo4_self_rel_many.json_object]], '$') IS {:TEST}) AND (NOT EXISTS (SELECT 1 FROM (SELECT JSON_EXTRACT([[__mm_demo4_self_rel_many.json_object]], '$') as [[multiMatchValue]] FROM `demo4` `__mm_demo4` LEFT JOIN json_each(CASE WHEN json_valid([[__mm_demo4.self_rel_many]]) THEN [[__mm_demo4.self_rel_many]] ELSE json_array([[__mm_demo4.self_rel_many]]) END) `__mm_demo4_self_rel_many_je` LEFT JOIN `demo4` `__mm_demo4_self_rel_many` ON [[__mm_demo4_self_rel_many.id]] = [[__mm_demo4_self_rel_many_je.value]] WHERE `__mm_demo4`.`id` = `demo4`.`id`) {{__smTEST}} WHERE NOT ([[__smTEST.multiMatchValue]] IS {:TEST})))) OR ((NULL IS JSON_EXTRACT([[demo4_self_rel_many.json_object]], '$')) AND (NOT EXISTS (SELECT 1 FROM (SELECT JSON_EXTRACT([[__mm_demo4_self_rel_many.json_object]], '$') as [[multiMatchValue]] FROM `demo4` `__mm_demo4` LEFT JOIN json_each(CASE WHEN json_valid([[__mm_demo4.self_rel_many]]) THEN [[__mm_demo4.self_rel_many]] ELSE json_array([[__mm_demo4.self_rel_many]]) END) `__mm_demo4_self_rel_many_je` LEFT JOIN `demo4` `__mm_demo4_self_rel_many` ON [[__mm_demo4_self_rel_many.id]] = [[__mm_demo4_self_rel_many_je.value]] WHERE `__mm_demo4`.`id` = `demo4`.`id`) {{__smTEST}} WHERE NOT (NULL IS [[__smTEST.multiMatchValue]])))) OR ((JSON_EXTRACT([[demo4_self_rel_many.json_object]], '$') IS JSON_EXTRACT([[demo4_self_rel_many.json_object]], '$')) AND (NOT EXISTS (SELECT 1 FROM (SELECT JSON_EXTRACT([[__mm_demo4_self_rel_many.json_object]], '$') as [[multiMatchValue]] FROM `demo4` `__mm_demo4` LEFT JOIN json_each(CASE WHEN json_valid([[__mm_demo4.self_rel_many]]) THEN [[__mm_demo4.self_rel_many]] ELSE json_array([[__mm_demo4.self_rel_many]]) END) `__mm_demo4_self_rel_many_je` LEFT JOIN `demo4` `__mm_demo4_self_rel_many` ON [[__mm_demo4_self_rel_many.id]] = [[__mm_demo4_self_rel_many_je.value]] WHERE `__mm_demo4`.`id` = `demo4`.`id`) {{__mlTEST}} LEFT JOIN (SELECT JSON_EXTRACT([[__mm_demo4_self_rel_many.json_object]], '$') as [[multiMatchValue]] FROM `demo4` `__mm_demo4` LEFT JOIN json_each(CASE WHEN json_valid([[__mm_demo4.self_rel_many]]) THEN [[__mm_demo4.self_rel_many]] ELSE json_array([[__mm_demo4.self_rel_many]]) END) `__mm_demo4_self_rel_many_je` LEFT JOIN `demo4` `__mm_demo4_self_rel_many` ON [[__mm_demo4_self_rel_many.id]] = [[__mm_demo4_self_rel_many_je.value]] WHERE `__mm_demo4`.`id` = `demo4`.`id`) {{__mrTEST}} WHERE NOT ([[__mlTEST.multiMatchValue]] IS [[__mrTEST.multiMatchValue]])))))",
},
}

Expand Down
Binary file modified tests/data/logs.db
Binary file not shown.
42 changes: 30 additions & 12 deletions tools/search/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,15 @@ func buildResolversExpr(
if !isAnyMatchOp(op) {
if left.MultiMatchSubQuery != nil && right.MultiMatchSubQuery != nil {
mm := &manyVsManyExpr{
leftSubQuery: left.MultiMatchSubQuery,
rightSubQuery: right.MultiMatchSubQuery,
op: op,
left: left,
right: right,
op: op,
}

expr = dbx.Enclose(dbx.And(expr, mm))
} else if left.MultiMatchSubQuery != nil {
mm := &manyVsOneExpr{
noCoalesce: left.NoCoalesce,
subQuery: left.MultiMatchSubQuery,
op: op,
otherOperand: right,
Expand All @@ -200,6 +201,7 @@ func buildResolversExpr(
expr = dbx.Enclose(dbx.And(expr, mm))
} else if right.MultiMatchSubQuery != nil {
mm := &manyVsOneExpr{
noCoalesce: right.NoCoalesce,
subQuery: right.MultiMatchSubQuery,
op: op,
otherOperand: left,
Expand Down Expand Up @@ -290,17 +292,29 @@ func resolveEqualExpr(equal bool, left, right *ResolverResult) dbx.Expression {
isRightEmpty := isEmptyIdentifier(right) || (len(right.Params) == 1 && hasEmptyParamValue(right))

equalOp := "="
nullEqualOp := "IS"
concatOp := "OR"
nullExpr := "IS NULL"
if !equal {
// use `IS NOT` instead of `!=` because direct non-equal comparisons
// always use `IS NOT` instead of `!=` because direct non-equal comparisons
// to nullable column values that are actually NULL yields to NULL instead of TRUE, eg.:
// `'example' != nullableColumn` -> NULL even if nullableColumn row value is NULL
equalOp = "IS NOT"
nullEqualOp = equalOp
concatOp = "AND"
nullExpr = "IS NOT NULL"
}

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

// both operands are empty
if isLeftEmpty && isRightEmpty {
return dbx.NewExp(fmt.Sprintf("'' %s ''", equalOp), mergeParams(left.Params, right.Params))
Expand Down Expand Up @@ -459,8 +473,8 @@ var _ dbx.Expression = (*concatExpr)(nil)
// concatExpr defines an expression that concatenates multiple
// other expressions with a specified separator.
type concatExpr struct {
parts []dbx.Expression
separator string
parts []dbx.Expression
}

// Build converts the expression into a SQL fragment.
Expand Down Expand Up @@ -503,16 +517,16 @@ var _ dbx.Expression = (*manyVsManyExpr)(nil)
// Expects leftSubQuery and rightSubQuery to return a subquery with a
// single "multiMatchValue" column.
type manyVsManyExpr struct {
leftSubQuery dbx.Expression
rightSubQuery dbx.Expression
op fexpr.SignOp
left *ResolverResult
right *ResolverResult
op fexpr.SignOp
}

// Build converts the expression into a SQL fragment.
//
// Implements [dbx.Expression] interface.
func (e *manyVsManyExpr) Build(db *dbx.DB, params dbx.Params) string {
if e.leftSubQuery == nil || e.rightSubQuery == nil {
if e.left.MultiMatchSubQuery == nil || e.right.MultiMatchSubQuery == nil {
return "0=1"
}

Expand All @@ -521,10 +535,12 @@ func (e *manyVsManyExpr) Build(db *dbx.DB, params dbx.Params) string {

whereExpr, buildErr := buildResolversExpr(
&ResolverResult{
NoCoalesce: e.left.NoCoalesce,
Identifier: "[[" + lAlias + ".multiMatchValue]]",
},
e.op,
&ResolverResult{
NoCoalesce: e.right.NoCoalesce,
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
Expand All @@ -538,9 +554,9 @@ func (e *manyVsManyExpr) Build(db *dbx.DB, params dbx.Params) string {

return fmt.Sprintf(
"NOT EXISTS (SELECT 1 FROM (%s) {{%s}} LEFT JOIN (%s) {{%s}} WHERE %s)",
e.leftSubQuery.Build(db, params),
e.left.MultiMatchSubQuery.Build(db, params),
lAlias,
e.rightSubQuery.Build(db, params),
e.right.MultiMatchSubQuery.Build(db, params),
rAlias,
whereExpr.Build(db, params),
)
Expand All @@ -556,10 +572,11 @@ var _ dbx.Expression = (*manyVsOneExpr)(nil)
//
// You can set inverse=false to reverse the condition sides (aka. one<->many).
type manyVsOneExpr struct {
otherOperand *ResolverResult
subQuery dbx.Expression
op fexpr.SignOp
otherOperand *ResolverResult
inverse bool
noCoalesce bool
}

// Build converts the expression into a SQL fragment.
Expand All @@ -573,6 +590,7 @@ func (e *manyVsOneExpr) Build(db *dbx.DB, params dbx.Params) string {
alias := "__sm" + security.PseudorandomString(5)

r1 := &ResolverResult{
NoCoalesce: e.noCoalesce,
Identifier: "[[" + alias + ".multiMatchValue]]",
AfterBuild: multiMatchAfterBuildFunc(e.op, alias),
}
Expand Down
8 changes: 7 additions & 1 deletion tools/search/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

func TestFilterDataBuildExpr(t *testing.T) {
resolver := search.NewSimpleFieldResolver("test1", "test2", "test3", `^test4_\w+$`)
resolver := search.NewSimpleFieldResolver("test1", "test2", "test3", `^test4_\w+$`, `^test5\.[\w\.\:]*\w+$`)

scenarios := []struct {
name string
Expand Down Expand Up @@ -93,6 +93,12 @@ func TestFilterDataBuildExpr(t *testing.T) {
false,
"[[test1]] NOT LIKE {:TEST} ESCAPE '\\'",
},
{
"nested json no coalesce",
"test5.a = test5.b || test5.c != test5.d",
false,
"(JSON_EXTRACT([[test5]], '$.a') IS JSON_EXTRACT([[test5]], '$.b') OR JSON_EXTRACT([[test5]], '$.c') IS NOT JSON_EXTRACT([[test5]], '$.d'))",
},
{
"macros",
`
Expand Down
5 changes: 5 additions & 0 deletions tools/search/simple_field_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ type ResolverResult struct {
// 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

// 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.
Params dbx.Params
Expand Down Expand Up @@ -99,6 +103,7 @@ func (r *SimpleFieldResolver) Resolve(field string) (*ResolverResult, error) {
}

return &ResolverResult{
NoCoalesce: true,
Identifier: fmt.Sprintf(
"JSON_EXTRACT([[%s]], '%s')",
inflector.Columnify(parts[0]),
Expand Down
2 changes: 1 addition & 1 deletion ui/.env
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ PB_DOCS_URL = "https://pocketbase.io/docs/"
PB_JS_SDK_URL = "https://github.com/pocketbase/js-sdk"
PB_DART_SDK_URL = "https://github.com/pocketbase/dart-sdk"
PB_RELEASES = "https://github.com/pocketbase/pocketbase/releases"
PB_VERSION = "v0.20.2"
PB_VERSION = "v0.20.3"

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4f24922

Please sign in to comment.