imdrasil / jennifer.cr

Crystal ORM using ActiveRecord pattern with flexible query DSL
MIT License
418 stars 55 forks source link

Model query hangs indefinitely with Postgres numeric[] columns #16

Open z64 opened 7 years ago

z64 commented 7 years ago

Hi there!

I've defined a model, and am simply trying to pull a full instance of it:

pp StarSystem.all.first!

However, this hangs forever with:

StarSystem.all.first! #=> 2017-07-07 23:25:49 -0400:   SELECT e.enumtypid
  FROM pg_type t, pg_enum e
  WHERE t.oid = e.enumtypid
2017-07-07 23:25:49 -0400: SELECT star_systems.*
FROM star_systems

This however works fine:

pp StarSystem.all.count
StarSystem.all.count # => 2017-07-07 23:27:18 -0400:   SELECT e.enumtypid
  FROM pg_type t, pg_enum e
  WHERE t.oid = e.enumtypid
2017-07-07 23:27:18 -0400: SELECT COUNT(*) FROM star_systems

1

This works too:

pp StarSystem.all.pluck(["name"]) 
StarSystem.all.pluck(["name"]) # => 2017-07-07 23:28:56 -0400:   SELECT e.enumtypid
  FROM pg_type t, pg_enum e
  WHERE t.oid = e.enumtypid
2017-07-07 23:28:56 -0400: SELECT star_systems.name
FROM star_systems

[["Njikan"]]

I resolved this issue at one point, but I can't remember how I did it :( According to the specs, this should work fine..

The postgres access logs look normal. I'm unsure, but it looks like it isn't actually running the query.

Something I'm missing?

imdrasil commented 7 years ago

will take a look today

imdrasil commented 7 years ago

@z64 it looks really strange. I've added several tests to match exact requirment and run all test suite with Logger::DEBUG but everything still works. Please paste code snippet to reproduce and crystal version.

z64 commented 7 years ago

Sure. A disclaimer, I've cleaned the verbose-ness of this a bit but haven't pushed the code yet (learning how to use this lib and PG at the same time! :smile: ) but this is basically what I'm working with:

Model: https://github.com/z64/traikoa/blob/master/src/traikoa/database/system.cr

Migration (again, mind my playing / learning, though maybe that's an issue): https://github.com/z64/traikoa/blob/master/src/traikoa/database/migrations/20170618001337825_system.cr

Config (it's default, from the README basically): https://github.com/z64/traikoa/blob/master/database_example.yml

Not the exact code I'm using, but close, and maybe something will jump out at you in the meantime until I get home later..

I'm using 0.23.

imdrasil commented 7 years ago

actually it is strange that you even have opportunity to run any code with jennifer using 0.23 crystal :smile: - they've changed macros inheritance strategy so you should get abstract def Jennifer::Model::Base#primary() must be implemented by Jennifer::Migration::Version exception (as me). This behavior is fixed (I have PR) but is waiting for 0.23.1 release (with fix for this one). But in general there is nothing that could cause this behavior. Add test case reproducing this and ping here.

z64 commented 7 years ago

Perhaps 0.22 then haha. again not at my home PC to check :(

And okay. I'll try to do some more digging later.. :sweat:

I'll also perhaps try running the current spec suite myself. I think that might be easier for me.

imdrasil commented 7 years ago

hi @z64 . Finally we get crystal 0.23.1 so we could continue on this one. Do you have any progress on this one?

z64 commented 7 years ago

Hey @imdrasil . The last time I worked on this I was trying to just run your existing specs for jennifer.cr to help narrow the issue, but I ran into problems getting it configured correctly to pass the first config spec - it failed with some long exception.

image

(this is with 0.22 with pg adapter)

I had set ENV["DB_USER"] and ENV["DB_PASSWORD"] correctly for my database and DB to postgres as described in config.cr.

Haven't had a moment to revisit it since, but if you have any ideas, the exception isn't very telling to me as to what might be wrong.

Also - good idea moving the README docs into the wiki! :+1:

imdrasil commented 7 years ago

@z64 it seems like you've missed password or user anyway - I've reproduced this removing setting them. Also values could be wrong or use has no rights to operate with this table. Please check it again and try to open psql with your creds.

imdrasil commented 7 years ago

@z64 any updates on this one?

z64 commented 7 years ago

Hey there! Sorry for the long delay :disappointed:

I've managed to run the specs after some fiddling with permissions on the postgres side that weren't right. It almost seemed like it was going to be related to #23 , as that's exactly what it does ; "freezes"

323 examples, 0 failures, 0 errors, 34 pending

That done, I'm unsure what else to look at.

The schema & model I'm currently working with: https://gist.github.com/z64/872ebab2293838cb627367a7a24bd236

And freezing with anything like:

StarSystem.where { _name == "Test" }.to_a

I've also tried the v_0.3.4 branch (specs pass on this too)

z64 commented 7 years ago

So, I took a step back and tried to reproduce the issue using only crystal-pg:

require "pg"

puts "Opening connection"
PG_DB = PG.connect("postgres://traikoa:traikoa@localhost")

puts "Creating table"
PG_DB.exec <<-SQL
CREATE TABLE IF NOT EXISTS array_test (
  position numeric[]
);
SQL

puts "Inserting values"
PG_DB.exec <<-SQL
INSERT INTO array_test (position) VALUES ('{0.1, 0.2, 0.3}');
SQL

puts "Running query"
PG_DB.query_one("select position from array_test") do |rs|
  puts rs
  puts rs.read(Array(Float64))
end

puts "Cleaning up"
PG_DB.exec <<-SQL
DROP TABLE array_test;
SQL

puts "Closing connection"
PG_DB.close

Hangs indefinitely. It would seem to be an issue with coercing numeric[] columns in the driver? Changing this to float[] fixes it :sweat_smile:

Seems I should take this issue to crystal-pg.

imdrasil commented 7 years ago

@z64 try it with PG::Numeric instead of Float64.

imdrasil commented 7 years ago

@z64 Did you try it?

z64 commented 7 years ago

I believe I did, and this threw other exceptions. Don't have access to them right this moment.

It seems like the supported array decoded types are defined here:

    {% for type in %w(Bool Char Int16 Int32 String Int64 Float32 Float64) %}
      def self.decode_array_element(io, t : {{type.id}}.class, dim_info)

https://github.com/will/crystal-pg/blob/master/src/pg/decoders/array_decoder.cr#L69

And later:

  array_type 1000, Bool
  array_type 1002, Char
  array_type 1005, Int16
  array_type 1007, Int32
  array_type 1009, String
  array_type 1016, Int64
  array_type 1021, Float32
  array_type 1022, Float64

https://github.com/will/crystal-pg/blob/master/src/pg/decoders/array_decoder.cr#L114-L121

which does not have Numeric or any other special handling for it.

I did open an issue too; https://github.com/will/crystal-pg/issues/105

imdrasil commented 7 years ago

@z64 hm, what do u think what we should do with this one here? There is no activity in the driver repo so I'm not sure fix for this will arrive soon.

z64 commented 7 years ago

Your call. Workarounds exist using float[] if you don't need the arbitrary precision or you can also use jsonb column ([1, 2.3, 4]). float[] works for my use case.

z64 commented 7 years ago

At the least, the issue is here and on the driver in case anyone runs into the same problem :sweat_smile:

imdrasil commented 7 years ago

I'm thinking what we should do with this - close it or leave opened. As for me this one is a missing funtionality from the pg driver and I can't do with this here nothing.