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

Read Array(Int) without considering database type #150

Open
vladfaust opened this issue Aug 17, 2018 · 7 comments
Open

Read Array(Int) without considering database type #150

vladfaust opened this issue Aug 17, 2018 · 7 comments

Comments

@vladfaust
Copy link
Contributor

vladfaust commented Aug 17, 2018

require "pg"

db = DB.open(ENV["DATABASE_URL"])
db.query_one("SELECT '{1,2}'::smallint[]") do |rs|
  puts rs.read(Array(Int16))
end

# => [1_i16, 2_i16]

This works, but if database schema defines int[] instead of smallint[], would return [0_i16, 4_i16]. Okay, not good. Let's try to fix it:

db.query_one("SELECT '{1,2}'::int[]") do |rs|
  puts rs.read(Array(Int16 | Int32))
end
# in lib/pg/src/pg/decoders/array_decoder.cr:66: no overload matches 'PG::Decoders.decode_array_element' with types IO+, (Int16 | Int32).class, Array(NamedTuple(dim: Int32, lbound: Int32))

Or maybe:

db.query_one("SELECT '{1,2}'::int[]") do |rs|
  puts rs.read(Array(Int16) | Array(Int32)); nil
end
# Unhandled exception: PG::ResultSet#read returned a Array(PG::Int32Array). A (Array(Int16) | Array(Int32)) was expected. (Exception)

Still no luck. What I want is to read an array of any integers and convert it to Int32 without taking care of what integer type is in DB schema. Is that possible?

Also rs.read(Int16 | Int32 | Int64 | Nil) works for any integer column type, so I think it should stay consistent.

@bcardiff
Copy link
Collaborator

Actual datatypes arr determined by the db/driver in the ResultSet#read. Types in #read(T) are used for better typing the crystal expression but the default implementation does not use that information when grabbing the value of the database.

The workaorund is to map/transform the data after the read operation.

@vladfaust
Copy link
Contributor Author

@bcardiff could you provide with an example of the workaround? Because it seems to me it doesn't solve the issue.

@vladfaust
Copy link
Contributor Author

@bcardiff I'm assuming that I don't know which integer type (smallint, int or bigint) is defined in DB schema and want to read its array. The issue is that neither rs.read(Array(Int16 | Int32)) nor rs.read(Array(Int16) | Array(Int32)) works. However, if dealing with non-array values, such a scenario is possible - rs.read(Int16 | Int32 | Int64 | Nil) - and it would work with any DB integer type.

@vladfaust
Copy link
Contributor Author

🏓

@asterite
Copy link
Contributor

I wonder whether some types should be automatically casted if there is no precision loss.

That means if you do rs.read(Int32) and the underlying type is Int8, Int16 or Int32 then it works out of the box. However if it's Int64 you would get a runtime error.

I don't think this is trivial to implement because we'd have to hardcode all these cases, but I'm pretty sure it'll be very useful to users.

When writing it should be the other way around: passing Int32 to a column that expects Int64 should be fine.

Now, thinking about these two cases, it might a bit weird that you can read Int32 from a column of type Int16, but when you try to write that value it will fail! So I'm actually not sure this is a good idea. An explicit conversion from an Array(Int16) to Array(Int32) (just doing rs.read(Array(Int16)).map(&.to_i32)) might be better because you can clearly see that you are reading from something that's Int16 but for some reason you are widening the type. Then if you get a failure you can at least track it and see "Ah ha!".

@RX14
Copy link
Contributor

RX14 commented Jul 23, 2019

This isn't a language problem - this is a problem in the design of crystal-db. JSON doesn't have this issue since it uses .from_json always, the same pull-parser design should be copied here to enable the type the user asked for to be an input to the parsing.

@straight-shoota
Copy link
Contributor

Reading values directly from result sets is a pretty low level interface. Specifying the type as returned from the database might just be the best solution.

Btw: Casting values to a specific type doesn't need to happen in Crystal application code, it could just be implemented in the SQL query as well. So in the end, it's just a matter of defining an interface between SQL and Crystal DB, while both sides could actually use different types internally.

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

5 participants