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

each method provides column values as strings rather than as their proper type #2

Closed
silasdavis opened this issue Jan 16, 2014 · 7 comments

Comments

@silasdavis
Copy link
Contributor

For example when using .each_row dates and integers are returned as strings in a hash. Also dynamic methods do not work, i.e. user.name does not work, but user[:name] does.

@RaVbaker
Copy link
Contributor

Please read documentation/README. .each_row returns values as hash with keys as ruby symbols. It doesn't know anything about type casting so it doesn't know how to convert date strings to Date objects and integer-like numbers to ints. If you want to have model objects use the .each_instance method. But then you have to deal with some drawbacks like a bit slower responses. Because casting costs.

@RaVbaker
Copy link
Contributor

I think this issue should be closed. cc: @afair

@silasdavis
Copy link
Contributor Author

I'd have to check (and I'm on the move now) but surely the underlying driver provide you bytes representing integers so you are serialising into a string somewhere which is presumably more costly than working with or as close to the native data type. I don't know about dates and so on. I'm not suggesting you return model values just that you honour the databases type system. This happens in most other database libraries I'm aware of.

@RaVbaker
Copy link
Contributor

I have reviewed code of postgresql_cursor but I don't see there any casting. I assume that it's a cause of the FETCH SQL statement which probably looses types of fields from the real query.

@afair
Copy link
Owner

afair commented May 14, 2014

Yes, .each_instance should provide the full instances type casting and methods. PostgreSQL returns all data as strings and the clients have to convert them into native data types.The pg gem may have a feature to do this, and I need to research it more.

My approach here has been to let .each_row pass along unaltered data for speed when crunching a large set of data, but this would be a nice in-between feature if it can be easily provided.

@silasdavis
Copy link
Contributor Author

I've looked into this, pg's exec_params (http://deveiate.org/code/pg/PG/Connection.html). It allows you to pass result_format = 1 to request binary rather than text. I'm not sure how activerecord handles this, but I would have thought it should surface the option to return binary as somtimes returning text (therefore encoding the binary) is wrong; when the data is some opaque binary format, although I guess this is a limited case. If you were to get the results in binary then you could in principle parse postgres integer into ruby integers for example.

There doesn't seem to be any functionality in the pg driver to do this for you (possible explained by this: http://wiki.postgresql.org/wiki/Driver_development#Type_Conversions) so it sounds like it might be too much heavy lifting for you to do in your layer.

@afair
Copy link
Owner

afair commented Jun 10, 2014

I'm closing this as described above. I studied the current ActiveRecord code, and think that using instances instead of rows is best for people who need Ruby types instead of strings.

Also, The PostgreSQL binary protocol doesn't transmit the data in Ruby types, and things like Fixnum and DateTime still need to be instantiated, and that's what AR (ActiveRecord) does anyway.

However, the current AR only performs these translations as needed and caches them to minimize the cost of instantiation. Because of that, I feel AR's work is really better than anything we can do here. Chasing the constantly changing inner working of AR is tough enough!

I have refactored the code in a v0.5 branch/release which I kept this issue in mind while working in it. I hope it works well for you.

@afair afair closed this as completed Jun 10, 2014
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

3 participants