jmoiron / modl

golang database modelling library
MIT License
479 stars 48 forks source link

Table mapped structs do not support embedded structs #13

Open silasdavis opened 10 years ago

silasdavis commented 10 years ago

If I attempt to map:

type CreatedUpdated struct {
    Created sql.NullInt64 //time.Time
    Updated sql.NullInt64 //time.Time
}

type ExternalSource struct {
    Uuid string
    Name string
    CreatedUpdated
}

With:

dbmap.AddTableWithName(ExternalSource{}, "external_sources")

It will fail with sql: converting Exec argument #2's type: unsupported type models.CreatedUpdated, a struct

This is because the embedded struct walking logic in dbmap.AddTable was removed with the intention to let sqlx handle the scanning.

Somehow we need to extract the logic in sqlx.StructScan and use it here, or if it does not generalise well add back in support for struct scanning in modl.

It also appears much of the bindPlan functionality from gorp has been abandoned, it would be nice to use prepared statements in the bind plan and also use the table mapping to remove as much reflection as possible when running a query. Since we have mapped the table up front I feel we should be able to use something leaner than sqlx.StructScan in Get or other planned queries...

silasdavis commented 10 years ago

Looking a bit deeper, it seems that we are not making as much use of TableMap as we could, chiefly in Get. But I wonder if we could aslo could maintain a set of 'TableMaps' (or an improved 'QueryMap') for all queries, possibly indexed by the query string or prepared statement.

jmoiron commented 10 years ago

It looks like I'm going to have to create a github.com/jmoiron/sqlx/reflect package so that the behavior of modl and sqlx can be kept in sync wrt handling more sophisticated object hierarchies. I had sort of planned on doing this anyway to get some reflect-type things (like sqlx.BaseSliceType, et al) out of the sqlx namespace. Doing this should make it easier to add this support back into the TableMap.

silasdavis commented 10 years ago

Okay let me know if there is any independent work that I could help with. I like the idea of extending/improving TableMap to shift as much computation/reflection to mapping time rather than query time. I'd also like to bind arbitrary non-table queries. Any thoughts on that?

sqs commented 10 years ago

Any update on this? I would be interested in adding support for embedded structs to modl, as I did for gorp. I'm happy to help you finish/test it if you've already begun on the sqlx/reflect package, or take your direction (if you have any plans) and implement it from scratch.

sqs commented 10 years ago

FWIW, I implemented support for embedded structs in https://github.com/sqs/modl/commit/e801ca1fef9784285e9a36cde3a01f7287c85f69. If you would like to use this implementation instead of refactoring and creating sqlx/reflect, I can submit a PR.

jmoiron commented 10 years ago

Please do, I'll almost certainly have time to look it over at Gophercon.

DylanLukes commented 9 years ago

Just sounding off, embedded structures would be great to have!

mattbostock commented 9 years ago

Likewise, I'd also like to see support for embedded structs.

maxhawkins commented 9 years ago

I'm also very interested in this feature.

Is there any way we can help, or is it dependent on new reflectx features?

ferhatelmas commented 9 years ago

Any news? Thanks!

nwidger commented 9 years ago

I'm interested in this as well. Any updates?

robert-zaremba commented 8 years ago

Any update here? We are also struggling with this.

dabfleming commented 8 years ago

Also just ran into this issue. Am currently using @sqs fork as a workaround.

Any chance of getting #17 merged?

robert-zaremba commented 8 years ago

It seams that this repo is not maintained any more. We also moved to @sqs 10 days ago.

sqs commented 8 years ago

FYI, we (@sourcegraph) just moved to go-gorp/gorp as we are not using embedded structs anymore. The @sqs fork has worked well for us, but I don't plan on maintaining it.

On Mar 11, 2016, at 01:39, Robert Zaremba notifications@github.com wrote:

It seams that this repo is not maintained any more. We also moved to @sqs 10 days ago.

— Reply to this email directly or view it on GitHub.