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

native extension method overloading incompatibility on 9K #2533

Closed
kares opened this issue Jan 28, 2015 · 2 comments
Closed

native extension method overloading incompatibility on 9K #2533

kares opened this issue Jan 28, 2015 · 2 comments
Labels
ir JRuby 9000
Milestone
Won't Fix

Comments

@kares
Copy link
Member

kares commented Jan 28, 2015

jruby 9.0.0.0-SNAPSHOT (2.2.0p0) 2015-01-22 0dae5d1 Java HotSpot(TM) 64-Bit Server VM 24.72-b04 on 1.7.0_72-b14 +jit [linux-amd64]

AR-JDBC's native RubyJdbcConnection.java class (historically) includes multiple versions of a table ruby method based on argument count. this worked fine on JRuby 1.7/1.6 but is no longer the case on 9K (guessing the last one with IRubyObject[] args "replaces" previous ones) :

    @JRubyMethod(name = "tables")
    public IRubyObject tables(ThreadContext context) {
        return tables(context, null, null, null, TABLE_TYPE);
    }

    @JRubyMethod(name = "tables")
    public IRubyObject tables(ThreadContext context, IRubyObject catalog) {
        return tables(context, toStringOrNull(catalog), null, null, TABLE_TYPE);
    }

    @JRubyMethod(name = "tables")
    public IRubyObject tables(ThreadContext context, IRubyObject catalog, IRubyObject schemaPattern) {
        return tables(context, toStringOrNull(catalog), toStringOrNull(schemaPattern), null, TABLE_TYPE);
    }

    @JRubyMethod(name = "tables")
    public IRubyObject tables(ThreadContext context, IRubyObject catalog, IRubyObject schemaPattern, IRubyObject tablePattern) {
        return tables(context, toStringOrNull(catalog), toStringOrNull(schemaPattern), toStringOrNull(tablePattern), TABLE_TYPE);
    }

    @JRubyMethod(name = "tables", required = 4, rest = true)
    public IRubyObject tables(ThreadContext context, IRubyObject[] args) {
        return tables(context, toStringOrNull(args[0]), toStringOrNull(args[1]), toStringOrNull(args[2]), getTypes(args[3]));
    }

on the Ruby side using this as @connection.tables(nil, current_schema) ends up with :

ArgumentError: wrong number of arguments (2 for 4)
/home/kares/workspace/oss/activerecord-jdbc-adapter/lib/arjdbc/derby/adapter.rb:403:in `tables'

is this a bug or a feature (in terms of native extension refactoring) on 9K ?

UPDATE:

also one can not work-around this using the following signature :

    @JRubyMethod(name = "tables")
    public IRubyObject tables(ThreadContext context, IRubyObject catalog, IRubyObject schemaPattern, IRubyObject tablePattern, IRubyObject types) {
         // ...
    }

... as it has never been supported by JRuby and remain the case on 9K :

java.lang.RuntimeException: Invalid specific-arity number of arguments (4) on method arjdbc.jdbc.RubyJdbcConnection.tables
    at org.jruby.internal.runtime.methods.InvocationMethodFactory$DescriptorInfo.<init>(InvocationMethodFactory.java:675)
    at org.jruby.internal.runtime.methods.InvocationMethodFactory.getAnnotatedMethodClass(InvocationMethodFactory.java:841)
    at org.jruby.internal.runtime.methods.InvocationMethodFactory.getAnnotatedMethod(InvocationMethodFactory.java:785)
    at org.jruby.RubyModule.defineAnnotatedMethod(RubyModule.java:816)
    at org.jruby.anno.TypePopulator$DefaultTypePopulator.populate(TypePopulator.java:104)
    at org.jruby.RubyModule.defineAnnotatedMethodsIndividually(RubyModule.java:808)
    at org.jruby.RubyModule.defineAnnotatedMethods(RubyModule.java:709)
    at arjdbc.jdbc.RubyJdbcConnection.createJdbcConnectionClass(RubyJdbcConnection.java:138)
    at arjdbc.jdbc.AdapterJavaService.basicLoad(AdapterJavaService.java:38)
@kares kares changed the title native extension incompatibility on 9K native extension method overloading incompatibility on 9K Jan 28, 2015
@enebo
Copy link
Member

enebo commented Jan 28, 2015

IR interpreter (e.g. ruby -> into ext) will only call boxed versions of methods currently. So it is always trying to dispatch to:

    @JRubyMethod(name = "tables", required = 4, rest = true)
    public IRubyObject tables(ThreadContext context, IRubyObject[] args) {

This is why it is 2 for 4 error. We have gotten lucky internally that we do not have a JRubyMethod signature like this (with required + rest) and we have made all IRubyObject[] args signatures always work for any submitted arity. This signature makes it clear we should just get this done properly.

@enebo enebo added this to the 9.0.0.0.pre2 milestone Jan 28, 2015
@headius
Copy link
Member

headius commented Apr 17, 2015

Do not specify required = 4 in the IRubyObject[] version. That forces all calls using that path to have at least 4 values. In general, the IRubyObject[] versions of methods should be able to handle all possible argument signatures.

We have run into this a few times before and there's really no good solution. We could make the invokers for [] try to redispatch specific-arity to the right paths but it's a lot more code generation in invoker generation.

Marking as won't fix.

@headius headius closed this as completed Apr 17, 2015
@headius headius modified the milestones: Won't Fix, 9.0.0.0.pre2 Apr 17, 2015
kares added a commit to jruby/activerecord-jdbc-adapter that referenced this issue Apr 20, 2015
@kares
seems that extension methods won't work in 9K the same as in 1.7.x th…
8747825 
…us work-around

originally reported as jruby/jruby#2533
kares added a commit to kares/activerecord-jdbc-adapter that referenced this issue May 20, 2015
@kares
seems that extension methods won't work in 9K the same as in 1.7.x th…
3ce43a5 
…us work-around

originally reported as jruby/jruby#2533
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ir JRuby 9000
Projects
None yet
Milestone
Won't Fix
Development

No branches or pull requests
3 participants
@headius @enebo @kares