#18 cannot convert nil to type (thing)

Closed
opened 2 years ago by robertmeta · 6 comments

Basically what it says on the tin, comparisons to

      if row.Date == nil { // ohh no, row.Date is a time.Time 
              return lib.Error("Preconditions failed, Date must be set.")
      }

same thing for mysql.Date type

Basically what it says on the tin, comparisons to if row.Date == nil { // ohh no, row.Date is a time.Time return lib.Error("Preconditions failed, Date must be set.") } same thing for mysql.Date type
otremblay commented 2 years ago
Owner

This is one of the not-fun ones; I can't easily decide what to do with this without wrapping mysql.Date and time.Time in nullable types (like database/sql's NullString), as pretty much everything has to be nullable as a mysql value.

I think it would make sense to expose nullable types everywhere, as this is also an issue in the Find functions; unless I use them, I can't differenciate in go between an empty string and "nothing" because a string's zero value is an empty string anyway.

This is one of the not-fun ones; I can't easily decide what to do with this without wrapping mysql.Date and time.Time in nullable types (like database/sql's NullString), as pretty much everything has to be nullable as a mysql value. I think it would make sense to expose nullable types everywhere, as this is also an issue in the Find functions; unless I use them, I can't differenciate in go between an empty string and "nothing" because a string's zero value is an empty string anyway.
otremblay commented 2 years ago
Owner

I started to work on this, I'm not sure how I want to deal with nil values. I ask for your advice, guys, what would make the most desirable API?

One that is language-idiomatic, meaning, I don't care about DB null values in the cases where Go doesn't natively deal with them?

Or one that is database-idiomatic, meaning, I go out of my way to make the DB code reflect the interaction with the DB more accurately? This entails making the building and usage of objects more complicated for the users of the generated API (more complicated structs generated from the tables).


Another way to go about this would be to generate a special "search" object or API thing, that could chain calls and perform reflection just for searches?

I'm thinking something along the following:

find.Where(X).Or(Y) // Takes two objects of a generated type, and combines them in a single "Where"
find.Where(find.IsNull("fieldname")) //Ugly, but what else can I do if I don't make use of nullable objects?
I started to work on this, I'm not sure how I want to deal with nil values. I ask for your advice, guys, what would make the most desirable API? One that is language-idiomatic, meaning, I don't care about DB null values in the cases where Go doesn't natively deal with them? Or one that is database-idiomatic, meaning, I go out of my way to make the DB code reflect the interaction with the DB more accurately? This entails making the building and usage of objects more complicated for the users of the generated API (more complicated structs generated from the tables). ------ Another way to go about this would be to generate a special "search" object or API thing, that could chain calls and perform reflection just for searches? I'm thinking something along the following: find.Where(X).Or(Y) // Takes two objects of a generated type, and combines them in a single "Where" find.Where(find.IsNull("fieldname")) //Ugly, but what else can I do if I don't make use of nullable objects?
otremblay commented 2 years ago
Owner

That being said, I DO expose "FindWithWhere".

That being said, I DO expose "FindWithWhere".

Well I would say that making things more complex to use and extend the generated API code is on the bad side, since at this point it seems like the majority of our use cases will center around using this application to generate a starting point for a more complicated API.

The reflection solution doesn't seem that ugly to me, but maybe that's just me.

Well I would say that making things more complex to use and extend the generated API code is on the bad side, since at this point it seems like the majority of our use cases will center around using this application to generate a starting point for a more complicated API. The reflection solution doesn't seem that ugly to me, but maybe that's just me.
otremblay referenced this issue from a commit 2 years ago
otremblay commented 2 years ago
Owner

Can you validate that this is fixed? I changed it so that it checks for "zeroed-out" time (see here: http://golang.org/pkg/time/#Time.IsZero) for time values.

Can you validate that this is fixed? I changed it so that it checks for "zeroed-out" time (see here: http://golang.org/pkg/time/#Time.IsZero) for time values.
otremblay commented 2 years ago
Owner

I'm closing this for now as the bug itself should not happen anymore (stuff I would have forgotten to check for as a type would have a "false" output in the "if", making it in effect unreachable, if I did not implement the type for the nullcheck).

I could open a new ticket for the reflective thing but at that point, it should be known that every db subpackage we generate has a "FindWithWhere" facility that has the same signature as database/sql.Query. It will prepare a statement with the "where" part of the sql query, and accept a list of params after that. I think it balances out both use cases where one would want to craft out a where clause by hand, or use the simpler api where you just affect an empty object with the things you wanna check against...

I'm closing this for now as the bug itself should not happen anymore (stuff I would have forgotten to check for as a type would have a "false" output in the "if", making it in effect unreachable, if I did not implement the type for the nullcheck). I *could* open a new ticket for the reflective thing but at that point, it should be known that every db subpackage we generate has a "FindWithWhere" facility that has the same signature as database/sql.Query. It will prepare a statement with the "where" part of the sql query, and accept a list of params after that. I think it balances out both use cases where one would want to craft out a where clause by hand, or use the simpler api where you just affect an empty object with the things you wanna check against...
Sign up for free to join this conversation. Already have an account? Sign in to comment
No Milestone
No assignee
Loading...
Cancel
Save
There is no content yet.