Skip to content
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

Cherry pick fix for panic reflect.Value.Interface on zero value #397

Merged

Conversation

davidhankins
Copy link
Contributor

This PR is a cherry-pick of PR #396 to the v0.25 branch:

git checkout tags/v0.25.4
git checkout -b cherry_pick_filter_nil
git cherry-pick 7670b4b57984186272ab7039733dff3da333b5fd

There were no conflicts to resolve.

PROBLEM
=======

In HTTP API services, a user may specify a `_filter=` specifying a
field name and an empty string (`field == ""`).

The `ToORM()` codec translates the empty string to a `nil` pointer.

`ProcessStringCondition()` then attempts to take a
`reflect.Value.Interface` on this nil pointer, causing a panic.

SOLUTION
========

Detect the nil pointer value, and return a nil value. The gorm codec
translates this to a `NULL` value in SQL syntax. This resolves the
panic, but results in an SQL query that matches no rows
(`field == NULL`).

DISCUSSION
==========

It's arguable what the user intent is here, considering that there is
existing Atlas filter syntax for `field == null` (resulting in the
correct identity SQL expression `field IS NULL`). The explicit empty
string value cannot exist in our interpretation because the codec
translates it to NULL reliably; a user that POSTs an object with a
string field set `""` will result in a table row set to NULL.

But we might want to consider that `field == ""` should be an alias for
the NULL identity expression; the user intent is probably to find rows
where the field is `""` in their experience of the object.

From one point of view, it is correct that there are never any rows in
our table with fields of `""` value. From another point of view, there
are rows in our table with fields not equal to `""` value and that query
should have results (the `field != ""` syntax seems like it should work).
@davidhankins
Copy link
Contributor Author

please rebuild

@davidhankins
Copy link
Contributor Author

oh that is probably the golang version difference

@daniel-garcia daniel-garcia merged commit 140df20 into infobloxopen:v0.25 Jan 9, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants