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

Premature parsing of json #125

Closed
cptaffe opened this issue Jan 17, 2018 · 4 comments · Fixed by #232
Closed

Premature parsing of json #125

cptaffe opened this issue Jan 17, 2018 · 4 comments · Fixed by #232

Comments

@cptaffe
Copy link

cptaffe commented Jan 17, 2018

To parse json into my own type using JSON.mapping, I have to do:

  def self.from_rs(rs : DB::ResultSet)
    from_json(rs.read(JSON::Any).to_json)
  end

This marshal/unmarshal thrashing could be avoided if pg just returned JSON::PullParser or some other type that doesn't scrap the ability to marshal via JSON.mapping

@asterite
Copy link
Contributor

I think we could do this automatically by asking whether T includes JSON::Serializable. It won't work with JSON.mapping but I think we should move forward with JSON::Serializable.

@cptaffe
Copy link
Author

cptaffe commented Jul 18, 2019

Is it possible to ask whether T provides from_json? That would work with JSON::Serializable and JSON.mapping as well as custom implementations of from_json.

The root issue IMO is that JSON::PullParser isn't an interface which JSON::Any could implement. For instance the Rust crate serde has serde_json::value::Value which is analogous to JSON::Any but it can be parsed into an object implementing the serde::Deserialize trait (similar to the from_json interface) using its from_value method .

@bcardiff
Copy link
Collaborator

Something else to consider regarding JSON::PullParser vs JSON::Any is whether the underlying Socket/IO is the one the PullParser is using, or if read will effectively duplicate it and leave it in memory. Using the same IO will limit the interface/protocol but will lead to better resource usage.

This is the problem I mean in crystal-db's readme as

Direct access to IO to avoid memory allocation for blobs.

This situation is similar to the one that appears when dealing with uploaded files in an https server. How regular params and the files can be parsed.

@z64
Copy link

z64 commented Jul 22, 2020

For those interested, an acceptable workaround for our application has been casting jsonb columns to TEXT in the query itself, to "bypass" the driver's handling of JSON::Any in this context:

require "pg"

record Foo, str : String, ints : Array(Int32) do
  include JSON::Serializable
end

db = PG.connect("postgresql:///")

res = db.query_all(<<-SQL, as: String).map { |json| Foo.from_json(json) }
SELECT meta::text
  FROM (VALUES
  ('{"str": "a", "ints": [1, 2, 3]}'::jsonb),
  ('{"str": "b", "ints": [4, 5, 6]}'::jsonb)
  ) t(meta)
SQL

p(res) # => [Foo(@ints=[1, 2, 3], @str="a"), Foo(@ints=[4, 5, 6], @str="b")]

You can make nice generic wrappers around this type of query for what is likely an acceptable cost.

Better still performance wise, you can pull JSON values out into the result set, from which you can use DB::Serializable:

record Bar, str : String, int : Int32 do
  include DB::Serializable
end

res = db.query_all(<<-SQL, as: Bar)
SELECT meta->>'str' as str, (meta->>'int')::integer as int
  FROM (VALUES
  ('{"str": "a", "int": 1337}'::jsonb),
  ('{"str": "b", "int": 1338}'::jsonb)
  ) t(meta)
SQL

p(res) # => [Bar(@int=1337, @str="a"), Bar(@int=1338, @str="b")]

Whether this is more or less practical, YMMV depending on use case

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

Successfully merging a pull request may close this issue.

4 participants