Skip to content

Implement $exists #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions filter/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,24 @@ func (c *Converter) convertFilter(filter map[string]any, paramIndex int) (string
v[operator] = c.arrayDriver(v[operator])
}
values = append(values, v[operator])
case "$exists":
// $exists only works on jsonb columns, so we need to check if the key is in the JSONB data first.
isNestedColumn := c.nestedColumn != ""
for _, exemption := range c.nestedExemptions {
if exemption == key {
isNestedColumn = false
break
}
}
Comment on lines +179 to +185
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is duplicated in the case nil: as well, we could lift it above the switch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can change those once the two pulls are both merged.

if !isNestedColumn {
// There is no way in Postgres to check if a column exists on a table.
return "", nil, fmt.Errorf("$exists operator not supported on non-nested jsonb columns")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can always return TRUE if it's not a nested column (or this could be an option).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's not true. The column can also not exist. The only way to actually check is selecting from information_schema.columns which seems dangerous and not possible since we don't know the table name.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, good point. 👍

}
neg := ""
if v[operator] == false {
neg = "NOT "
}
inner = append(inner, fmt.Sprintf("(%sjsonb_path_match(%s, 'exists($.%s)'))", neg, c.nestedColumn, key))
default:
value := v[operator]
op, ok := basicOperatorMap[operator]
Expand Down
40 changes: 24 additions & 16 deletions filter/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,6 @@ func TestConverter_Convert(t *testing.T) {
nil,
fmt.Errorf("invalid value for $not operator (must be object): John"),
},
{
"sql injection",
nil,
`{"\"bla = 1 --": 1}`,
``,
nil,
fmt.Errorf("invalid column name: \"bla = 1 --"),
},
{
"compare with array",
nil,
Expand All @@ -264,14 +256,6 @@ func TestConverter_Convert(t *testing.T) {
nil,
fmt.Errorf("invalid comparison value (must be a primitive): [200 300]"),
},
{
"sql injection",
nil,
`{"\"bla = 1 --": 1}`,
``,
nil,
fmt.Errorf("invalid column name: \"bla = 1 --"),
},
{
"null nornal column",
nil,
Expand All @@ -288,6 +272,30 @@ func TestConverter_Convert(t *testing.T) {
nil,
nil,
},
{
"$exists on normal column",
nil,
`{"name": {"$exists": false}}`,
``,
nil,
fmt.Errorf("$exists operator not supported on non-nested jsonb columns"),
},
{
"not $exists jsonb column",
filter.WithNestedJSONB("meta"),
`{"name": {"$exists": false}}`,
`(NOT jsonb_path_match(meta, 'exists($.name)'))`,
nil,
nil,
},
{
"$exists jsonb column",
filter.WithNestedJSONB("meta"),
`{"name": {"$exists": true}}`,
`(jsonb_path_match(meta, 'exists($.name)'))`,
nil,
nil,
},
}

for _, tt := range tests {
Expand Down
1 change: 1 addition & 0 deletions fuzz/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func FuzzConverter(f *testing.F) {
`{"$not": {"name": "John"}}`,
`{"name": {"$not": {"$eq": "John"}}}`,
`{"name": null}`,
`{"name": {"$exists": false}}`,
}
for _, tc := range tcs {
f.Add(tc, true)
Expand Down
12 changes: 12 additions & 0 deletions integration/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,18 @@ func TestIntegration_BasicOperators(t *testing.T) {
[]int{10},
nil,
},
{
`jsonb exists`,
`{"pet": {"$exists": false}}`,
[]int{9},
nil,
},
{
`jsonb exists`,
`{"pet": {"$exists": true}}`,
[]int{1, 2, 3, 4, 5, 6, 7, 8, 10},
nil,
},
}

for _, tt := range tests {
Expand Down
Loading