Remove runtime reflection introduced in v1.5
AlekSi opened this issue · 0 comments
Since at least reform v1.0.0, it generated the following code:
Lines 127 to 134 in 213ddec
Type
there is a Record struct
type, PKField.Name
is record's primary key field name, int64 is a fixed type returned by database/sql.Result.LastInsertId, and PKField.Type
is an actual primary key filed type as written in the source file. The first branch is used for databases and drivers that use autoincrement integer primary keys via LastInsertId. The second branch is used for other databases like PostgreSQL, where RETURNING
clause is used for that. Reform itself actually uses that generated method only in one place:
Lines 86 to 99 in 213ddec
Effectively, only the first branch is actually used, but SetPK was kept in the current form for generability. In retrospect, that was a mistake: go vet
in 1.15 started to complain about that code as reported at #245 due to golang/go#32479.
I investigated three ways to fix that problem:
- Significantly change the behavior of this method, change the signature, or completely remove it. That would be a breaking change. It would be for the better, but I still don't want to make it in the v1 module and don't want to make v2 now.
- Get more information about the field type using static file parsing with go/types and related packages to generate branchless code: #246
- Get more information about the field type from the runtime during program initialization and use it: #249
The latter two approaches turned out to be complicated and time-consuming for custom types, types from other packages, type aliases, etc. So I released v1.5.0, which uses runtime reflection for setting the primary key: #266 It should be changed to approach 2 or 3 in v1.6.0.