omitempty bug in InsertRecords

[ScreamingHawk]

When you have a table with a field tagged omitempty, calling InsertRecords will fail if some records have empty values and some do not. The error is pgkit: 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

[david-littlefarmer]

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 NULL.

I think this is valid definition.

type Account struct {
	...
	AltName   *string   `db:"alt_name"` // can be NULL
	...
}

So never to have pointer and omitempty on one field.

Also i think that omitempty is used only for values, that are set by DB, as created at timestamp or/and id as primary key assigned by DB.

Maybe i am completely wrong 😅

But yes, this is still a bug i guess.

[VojtechVitek]

@david-littlefarmer what if you want to store empty string?

[david-littlefarmer]

You mean like this?

str := ""
o.AltName = &str

[LukasJenicek]

If I wanted store empty string then I would never made the column nullable and it should be required at least empty string.

So in the code it would not be pointer as well.

[ScreamingHawk]

I've updated the example to something more realistic. Setting created_at manually for one and expecting the db to populate with DEFAULT CURRENT_TIMESTAMP on the other.

[pkieltyka]

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

[VojtechVitek]

#23 showcased the following bug with batch insert statements:

ERROR:  VALUES lists must all be the same length at character 68
STATEMENT:  INSERT INTO accounts (created_at,disabled,name) VALUES ($1,$2,$3),($4,$5),($6,$7,$8)

Batch INSERT statement can fail if the values passed have both non-zero and zero field value with ,omitempty tag.

[VojtechVitek]

Fixed by #54 + #61.