modelop / hadrian

Implementations of the Portable Format for Analytics (PFA)
Apache License 2.0
130 stars 49 forks source link

Hadrian type-safe cast fails on ["null", "int"] #23

Open nrpeterson opened 7 years ago

nrpeterson commented 7 years ago

I've got the following simple file, consisting only of a type-safe cast to return a boolean for whether or not the input is non-null:

{
  "input": ["null", "int"],
  "output": "boolean",
  "action": [
    {
      "cast": "input",
      "cases": [
        {
          "as": "null",
          "named": "x",
          "do": [false]
        },
        {
          "as": "int",
          "named": "i",
          "do": [true]
        }
      ]
    }
  ]
}

Calling this in Hadrian with null as the input throws the following exception:

Exception in thread "main" scala.MatchError: null
    at com.opendatagroup.hadrian.jvmcompiler.W$.asInt(jvmcompiler.scala:413)
    at com.opendatagroup.hadrian.jvmcompiler.W.asInt(jvmcompiler.scala)
    at PFA_Engine_1$2$1$2.apply(Unknown Source)
    at PFA_Engine_1$2$1.apply(Unknown Source)
    at PFA_Engine_1$2.apply(Unknown Source)
    at PFA_Engine_1.action(Unknown Source)
    at PFA_Engine_1.action(Unknown Source)
    at Main$.main(Main.scala:16)
    at Main.main(Main.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

If I switch it up and use the "ifnotnull" special form instead, it works just fine.

When I switch the datatype from ["null", "int"] to ["null", "double"], it still fails. When I switch it to ["null", "string"], the model runs... but produces the wrong output (it says true).

Is there an issue here of pattern matching against null?

(I should add: I have verified that null is being interpreted as null, and not as "null")

jpivarski commented 7 years ago

This does look like an error, thanks. Ifnotnull is supposed to be a convenient synonym for cast-cases (especially convenient when the number of variables you want to check is large), so their behavior should have been the same.

null in Scala's pattern matching often has to be a special case, and the first special case, since Scala's official position is that we shouldn't be using null (but Avro generates them). However, the real error is probably a few lines upstream of where this threw an exception, in the Java code generated by jvmcompiler.scala for the cast-case block. It should have matched "null" by an explicit Java if statement.