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.

Sign up for GitHub

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

Jump to bottom

Regression in method calling on Java objects in 9.0.5.0 (field not calling getField for JDBC object) #3622

Closed
jeremyevans opened this issue Jan 27, 2016 · 5 comments
Closed

Regression in method calling on Java objects in 9.0.5.0 (field not calling getField for JDBC object) #3622

jeremyevans opened this issue Jan 27, 2016 · 5 comments
Labels
java integration
Milestone
Won't Fix

Comments

@jeremyevans
Copy link
Contributor

Running Sequel's specs with jdbc-postgres on JRuby 9.0.5.0 results in nearly all failures of the type:

  1) Error:
Dataset UNION, EXCEPT, and INTERSECT#test_0002_should give the correct results for UNION, EXCEPT, and INTERSECT when used with ordering and limits:
NoMethodError: undefined method `field' for #<Java::OrgPostgresqlJdbc4::Jdbc4ResultSetMetaData:0x675bf541>
    L:/sequel/lib/sequel/adapters/jdbc/postgresql.rb:219:in `type_convertor'
    L:/sequel/lib/sequel/adapters/jdbc.rb:800:in `block in process_result_set'
    org/jruby/RubyFixnum.java:296:in `times'
    L:/sequel/lib/sequel/adapters/jdbc.rb:798:in `process_result_set'
    L:/sequel/lib/sequel/adapters/jdbc.rb:737:in `block in fetch_rows'
    L:/sequel/lib/sequel/adapters/jdbc.rb:253:in `block in execute'
    L:/sequel/lib/sequel/adapters/jdbc.rb:674:in `statement'
    L:/sequel/lib/sequel/adapters/jdbc.rb:248:in `block in execute'
    L:/sequel/lib/sequel/connection_pool/threaded.rb:105:in `hold'
    L:/sequel/lib/sequel/database/connecting.rb:256:in `synchronize'
    L:/sequel/lib/sequel/adapters/jdbc.rb:247:in `execute'
    L:/sequel/lib/sequel/dataset/actions.rb:952:in `execute'
    L:/sequel/lib/sequel/adapters/jdbc.rb:737:in `fetch_rows'
    L:/sequel/lib/sequel/dataset/actions.rb:1056:in `returning_fetch_rows'
    L:/sequel/lib/sequel/dataset/actions.rb:336:in `insert'
    L:/sequel/lib/sequel/adapters/shared/postgres.rb:1351:in `insert'
    L:/sequel/lib/sequel/adapters/shared/postgres.rb:1360:in `insert'
    L:/sequel/spec/integration/dataset_test.rb:580:in `block in (root)'
    org/jruby/RubyBasicObject.java:1633:in `instance_eval'

All tests pass in JRuby 9.0.4.0. field should work here, calling the getField Java function, see the PostgreSQL JDBC API: https://jdbc.postgresql.org/development/privateapi/org/postgresql/jdbc4/Jdbc4ResultSetMetaData.html

Changing the ruby code to call getField instead of field (and also getOID instead of oid on the object returned by getField) does fix the issues, but my understanding is that JRuby should work with field and oid, and that this is a regression in JRuby.
@jeremyevans jeremyevans mentioned this issue Jan 27, 2016
Closed
@kares
Copy link
Member

kares commented Jan 27, 2016

@jeremyevans this is kind of intended ... see discussion at: #3262
we hoped no one uses indexed getters such as in your case getField(int) - could you refactor, pls?
a getField or get_field will work in all previous versions as well as 9.0.5.0 ... but if there's someone really demanding it back we shall add a switch to enable these. really sorry for the inconvenience ...

@jeremyevans
Copy link
Contributor Author

I've already committed a work around to Sequel about a half hour ago. I don't disagree with the logic in #3262 in regards to removing the feature, but I think it may have been better to do it in 9.1 as opposed to 9.0.5.0, especially since this was not mentioned as a backwards compatibility issue in the release notes. It is true that #3262 is mentioned in the release notes, but unless you expect all users to read every referenced note, it's unlikely someone would think it affected backwards compatibility.

@kares
Copy link
Member

kares commented Jan 27, 2016

yeah, did a really bad job around this one. simply got carried away that it can not be used as the writer version would look completely weird and the feature wasn't documented at all (only normal getter -> reader mappings were used in samples). sorry again - backwards compat is crucial and this should have printed a deprecation warning first. and as you said removal would have been fine in 9.1.0 .... 😭

@kares kares added this to the Won't Fix milestone Jan 27, 2016
@abrandoned abrandoned mentioned this issue Feb 10, 2016
Closed
@PragTob
Copy link

PragTob commented Feb 21, 2016

This just bit us too over in shoes/shoes4#1217 (cc @plexus) I guess I'll check the Mailing list and notify it too if that hasn't happened yet :|

Yep 9.1.0 would have been better, but there's not much good in regrets :) Also in favor of the change, was a bit too much magic maybe.

Cheers + thanks for your work as always,
Tobi

@headius headius closed this as completed Mar 4, 2016
@donv
Copy link
Member

donv commented Mar 27, 2016

Just one more "me too" 😄 We had to rewrite our use of the POI API. row(10) and cell(42) had to be changed to get_row(10) and get_cell(42). Quickly done, and no problem. Just unexpected for a patch level release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
java integration
Projects
None yet
Milestone
Won't Fix
Development

No branches or pull requests
5 participants
@jeremyevans @headius @kares @donv @PragTob