-
Notifications
You must be signed in to change notification settings - Fork 4
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
omitempty bug in InsertRecords #24
Comments
This doesn't make much sense to me: type Account struct {
...
AltName *string `db:"alt_name,omitempty"` // can be NULL
...
} When you have pointer to a string and nullable field in DB, then with omitempty you wouldn't be ever able to set the field in DB from some value to I think this is valid definition. type Account struct {
...
AltName *string `db:"alt_name"` // can be NULL
...
} So never to have pointer and Also i think that Maybe i am completely wrong 😅 But yes, this is still a bug i guess. |
@david-littlefarmer what if you want to store empty string? |
You mean like this? str := ""
o.AltName = &str |
If I wanted store empty string then I would never made the column So in the code it would not be pointer as well. |
I've updated the example to something more realistic. Setting |
can you guys check the https://github.com/goware/pgkit/blob/master/tests/pgkit_test.go to see if the case is covered..? maybe its just more confusing on how to use it, versus a bug..? as well, if you do determine a bug, please add a failing test case in a PR, the test suite is pretty good and easy to work with, and this is the schema btw: https://github.com/goware/pgkit/blob/master/tests/schema_test.go once we have a failing test case, we can look |
When you have a table with a field tagged
omitempty
, callingInsertRecords
will fail if some records have empty values and some do not. The error ispgkit: ERROR: VALUES lists must all be the same length (SQLSTATE 42601)
.This is because the reflection (correctly) ignores the fields tagged
omitempty
, which results in different fields being supplied for the insert statement.https://github.com/goware/pgkit/blob/master/builder.go#L51
https://github.com/goware/pgkit/blob/master/mapper.go#L41
Example test case: #23
The text was updated successfully, but these errors were encountered: