Generates IN/NIN whereHelpers for nullable types

[bbarcklay]

Enables IN/NIN whereHelper function generation for nullable types addressing issue #851. While this implements the pattern established by the other functions, I would find it more convenient to pass regular strings or ints for the IN/NIN tests. Thoughts?

[aarondl]

While this implements the pattern established by the other functions, I would find it more convenient to pass regular strings or ints for the IN/NIN tests. Thoughts?

I think that this is the reason these don't exist in the first place. The "right" answer is a little bit hard to find. Either way you're going to end up with conversions at some point. And it's also not clear that the code here is what we want because of the trap of feeding a null to the in clause which is usually probably not what you wanted.

If you go the way of only being able to provide ints, then you limit the ability to create a simple list of these with something like:

models.Model(models.ModelWhere.Column.IN(otherModel1.X, otherModel2.X))

Of course that comes with a potential trap of generating IN (5, null) which you might want to restrict in some way because does anyone ever actually want that? What's the least surprising behavior here?

[fdegiuli]

If you go the way of only being able to provide ints, then you limit the ability to create a simple list of these with something like:

models.Model(models.ModelWhere.Column.IN(otherModel1.X, otherModel2.X))

My gut feeling is that it's probably okay for this to not be supported. Given that passing a NULL to a WHERE IN clause is largely meaningless in SQL, it seems reasonable not to accept nullable types in the helper to generate that clause.
You do miss out on some convenience, but anybody currently manually creating IN clauses wouldn't be any worse off, nor does this preclude us from adding such functionality at a later date.

Of course that comes with a potential trap of generating IN (5, null) which you might want to restrict in some way because does anyone ever actually want that? What's the least surprising behavior here?

Not accepting null types would handle this. Also, IN (5, NULL) is valid, if pointless, SQL; and I would be fairly surprised to see my ORM doing nil checks for me.

Either way you're going to end up with conversions at some point.

This is a little annoying, but the null.T types are trivially mappable to their underlying go types. A toPrimitiveType(string) string function would be easy to write. With decent test coverage this should be pretty safe.
Let me know if you want me to take a crack at it.

[aarondl]

This will require some rebasing (bindata has been removed and there are changes in the same area of the code). I'm still largely undecided on it and lacking compelling arguments one way or the other I'm not sure what we should be doing here.

I think the biggest obstacle to overcome in saying we should just accept primitives like Age.IN(5, 6, 7) is that you have to do some additional mapping to figure out the null types non-null type?

Food for thought.

[fdegiuli]

I think the biggest obstacle to overcome in saying we should just accept primitives like Age.IN(5, 6, 7) is that you have to do some additional mapping to figure out the null types non-null type?

Yeah, I mentioned it above but I imagine something like this would be fairly safe and straightforward, given that primitive types aren't likely to change much:

func toPrimitiveType(string t) string {
  switch t {
  case "null.String": 
    return "string"
  case "null.Int":
    return "int"
  case "null.Int64":
    return "int64"
  ...
  }
}

[stephenafamo]

Kindy reopen this PR to master, it was mistakenly closed when the dev branch was deleted.