Skip to content

WIP: Support Filtering using ne:null on nullable attribute #386

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

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Aug 28, 2018

BUG FIX

  • reproduce issue in tests
  • fix issue
  • bump package version

The root of this was that I wanted to be able to ?filter[nullable-field]=ne:, meaning that I wanted all resources where nullable-field had a value.

I had originally tried using ?filter[nullable-field]=ne:null but, this was interpreting null as "null", which resulted in the error: Could not cast null to Nullable'1.

Since checking if something is not null can be handy, I added a check for the Nullable generic type, and if the value is an empty string, swap it out for null. :-\

This may be a breaking change. idk.

@NullVoxPopuli NullVoxPopuli changed the title Support Filtering using ne:null on nullable attribute WIP: Support Filtering using ne:null on nullable attribute Aug 28, 2018
@NullVoxPopuli
Copy link
Contributor Author

what if I add two FilterOperations for notnull and @null?

@milosloub
Copy link
Contributor

milosloub commented Aug 28, 2018

@NullVoxPopuli good idea! But this issue with null is quite confusing.
In case of ?filter[nullable-attr]=notnull (or null) and its FilterOperation (notnull or @null)
leads to Resource.NullableAttr != "notnull". It's because of default eq operator, when there is no
FilterOperation provided. So to fix this, you should have route like this:
?filter[nullable-attr]=notnull:someIrrelevantValue.

I think you were right with previous solution ?filter[nullable-attr]=NULL or filter[nullable-attr]=ne:NULL.

Edit: For error Could not cast null to Nullable'1,
instead of this:
var value = isNullabe && propertyValue == "" ? null : propertyValue;
try:
var value = isNullabe && propertyValue == "" ? (property.PropertyType)null : propertyValue;

Ternary operator should return same type of value.

@NullVoxPopuli
Copy link
Contributor Author

I think you were right with previous solution ?filter[nullable-attr]=NULL or filter[nullable-attr]=ne:NULL

but what if someone wants to test for the string value of "NULL"

@NullVoxPopuli
Copy link
Contributor Author

Maybe something like this?
image
(just now value on the right side of the colon)

@milosloub
Copy link
Contributor

Yeah, thats the problem of string stored NULL values.
What about something like filter[attr]=is:null and filter[attr]=isnot:null?
This is similar to SQL queries

@NullVoxPopuli
Copy link
Contributor Author

what's to stop the usage of is:somethingelse? :-\ would we hard-code checks on null value usage?

@milosloub
Copy link
Contributor

Here is null value like every other value after colon. So there must be conversion to null in this case. Maybe someone comes with better idea.
In case of is:somethingElse...its should be the same as eq:somethingElse. I didnt try the difference between Items.Where(i => i.Attr == value) and Items.Where(i => i.Attr.Equals(value)) from EF Core point of view.
I dont know if EF core translate Items.Where(i => i.Attr.Equals(null)) to SQL: "WHERE Attr IS NULL" by default.
If yes, operator "is" or "isnot" can be used to build Expression based od Equals Method.

@NullVoxPopuli
Copy link
Contributor Author

cool. I'll submit a different PR with is and isnot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants