Skip to content
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

StructScan Unmarshalling Behaviour: no fields in dest. interface #45

Closed
elithrar opened this issue Jan 24, 2014 · 9 comments
Closed

StructScan Unmarshalling Behaviour: no fields in dest. interface #45

elithrar opened this issue Jan 24, 2014 · 9 comments

Comments

@elithrar
Copy link
Contributor

Is there a way to change the behaviour (default or optional) of StructScan to match that of Go's json.Unmarshal where any fields in the destination struct that don't exist in the source are implicitly skipped?

Example: I have a tsv field that contains tsvector data in a postgres DB for full text search. I can avoid having a matching field in the destination struct if I SELECT <list of columns not including tsv however it's pretty verbose once you have a few columns and it'd be easier to just SELECT * FROM <table> and drop the columns not in the dest struct.

@jmoiron
Copy link
Owner

jmoiron commented Jan 24, 2014

There's no API in sql to scan only certain columns in a result set; the underlying function is Scan(interface{}, ...); so I'd have to make up some datatype which accepts everything and use it to scan the empty columns. I think I could define such a type in sqlx.types; while the driver still has to read it (it probably does anyway), at least such a type could avoid making a copy.

I'm in two minds with this. StructScan is quite like a Marshaller, and it's descendent from Scan, which doesn't throw errors if you don't give enough arguments to it. On the other, I think this is much more often a programming error, especially starting out (db tags wrong, it being slightly less than obvious that the FieldMapper lowercases everything by default).

I know this makes SQL clumsy to write, and I know that the current answer for "What happens when we need to migrate our schema?" is very sub-optimal, but sqlx isn't the place to address these issues. Behaving more in line with expectations is a more compelling argument.

@kisielk
Copy link
Contributor

kisielk commented Jan 24, 2014

Recently someone sent a patch in to sqlstruct that enables something like this, allowing you to scan one query in to multiple structs: kisielk/sqlstruct@7814118

@elithrar
Copy link
Contributor Author

@jmoiron Fair enough. I'm curious as to where the programmer error might be in this case: the tsv column is effectively (Postgres specific) metadata that I wanted to divorce from my application code.

Struct tags won't help here as I can't effectively "ignore" a column coming from the database (only from my app to it), as far as I understand it.

I do get your reasoning though, as not everything must behave like a Marshaller ;)

@kisielk
Copy link
Contributor

kisielk commented Jan 25, 2014

You can "ignore" columns in a scan by decoding in to sql.RawBytes, assuming you know which columns you want to decode ahead of time. That's what sqlstruct does.

jmoiron added a commit that referenced this issue Apr 24, 2014
…y; unfortunately RawBytes cannot be used in Row.Scan
@jmoiron
Copy link
Owner

jmoiron commented Apr 24, 2014

Alright, I've made a shot at this, because behaving like other marshallers is what sqlx should have done from the beginning.

Because Row.Scan and Rows.Scan currently share all of this code, I cannot use sql.RawBytes (yet), so I have to fall back on *[]byte, which works well but copies from the driver.

Unfortunately, even this doesn't work for everything; lib/pq's time.Time support rather confuses things, so leaving out timestamps will fail.

@jmoiron
Copy link
Owner

jmoiron commented Apr 24, 2014

Ah, *interface{} works for everything. Since []byte already has to copy anyway it's not any worse. This seems to work as intended.

@jmoiron
Copy link
Owner

jmoiron commented Apr 25, 2014

After some discussion in #go-nuts yesterday, I'm having second thoughts on making this a new default behavior. Although it is analogous to the way the marshalers work, you generally have full control over what SQL you are executing. It seems odd that sqlx will cover you for incompletely modeling your data.

Despite that, I think there are cases where this still might be useful. If you are using some kind of criteria for sorting which is not in your actual data model, it can be cumbersome to create a new struct for that (even with embedding). Still, this feels like a case where in 99% of cases the fault is some kind of laziness of the programmer, and not some intractable lack of information.

I'm content to make this an option, but the way I handled NameMapper is in retrospect very poor. I was striving for convenience, but in the end if a library is using sqlx to, say, store some configuration in sqlite3, and it sets NameMapper, you can break it by setting it yourself. This is the curse of shared global state and I should have known better.

I'm not sure where this option might belong. It'd be nice to do this in an isolated way for only one particular query, but still allow the possibility to enable it wider for people who, against the warnings of others, want or need this as a general thing.

The interface I'd like would be a new method to sqlx.DB and sqlx.Tx, something like:

def (d *DB) Unsafe() *DB {
    // clone DB, set an unsafe flag, and return it
}

This object would then be using the unsafe/unstrict behavior.

This would involve some type switching to get at the actual flag, and then passing it down the line to the reflect code, but I think it should work without disruption to the public API.

@jmoiron
Copy link
Owner

jmoiron commented Apr 26, 2014

The way to do this is now described in the readme.

@jmoiron jmoiron closed this as completed Apr 26, 2014
@DenLilleMand
Copy link

DenLilleMand commented Sep 19, 2018

I personally came to this issue because have some inheritance mapped in the DB using the newest pgsql feature for it, and was looking for a way to easily load the hierarchy from the DB into a single struct.

I just realized that you can embed structs inside of each other anonymously which is MUCH prettier. Likely no-one will be in the exact same situation as me, just wanted to suggest that as an alternative to the lazy programmers out there.

EDIT: Sorry for necro bumping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants