Uploaded image for project: 'Drools'
  1. Drools
  2. DROOLS-2814

MVEL uses random parameter type methods for BigDecimal.valueOf(100)

    • Icon: Bug Bug
    • Resolution: Done
    • Icon: Major Major
    • None
    • 7.9.0.Final
    • core engine
    • 2018 Week 30-32
    • Hide

      See Description

      Show
      See Description
    • NEW
    • NEW

      When MVEL evaluates

      java.math.BigDecimal.valueOf(100)
      

      , MVEL determines a method from valueOf(double val) or valueOf(long val).

      The result is not consistent. Sometimes uses valueOf(double val), Sometimes uses valueOf(long val).

      ParseTools.getMethodScore() gives the same score for double and long.

      https://github.com/mvel/mvel/blob/master/src/main/java/org/mvel2/util/ParseTools.java#L312-L367

      So a selected method will depend on the order of result of "cls.getMethods()".

      Reproducer:

      $ unzip BigDecimalMvelTest.zip
      $ cd BigDecimalMvelTest
      $ mvn compile
      $ ./loop.sh

      It runs the MVEL test 100 times:

      $ ./loop.sh 
      result = 100.0
      result = 100.0
      result = 100.0
      result = 100
      result = 100.0
      result = 100.0
      ...
      

      In my environment, "100.0" is the majority but randomly see "100" as well. (I couldn't reproduce with single run with java for loop)

      Expectation is constantly "100" (using valueOf(long val)) because in case of usual Java "java.math.BigDecimal.valueOf(100)", Java compiler chooses valueOf(long val) if the parameter is int. (considering "direct supertype" https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.10)

            [DROOLS-2814] MVEL uses random parameter type methods for BigDecimal.valueOf(100)

            Mario Fusco added a comment - - edited

            Since we use a clone of the same algorithm also in the executable model I fixed it also there with this commit https://github.com/kiegroup/drools/commit/067aa33e9d322d6c8e6874b08c226cdb1d3032a3

            Mario Fusco added a comment - - edited Since we use a clone of the same algorithm also in the executable model I fixed it also there with this commit https://github.com/kiegroup/drools/commit/067aa33e9d322d6c8e6874b08c226cdb1d3032a3

            Mario Fusco added a comment - Fixed in mvel with https://github.com/mvel/mvel/commit/0283c28e0a61f4747c86442ce6663bbd5485a3dc

            Just wanted to mention, this analysis is done by mkobayas. Thanks!

            Toshiya Kobayashi added a comment - Just wanted to mention, this analysis is done by mkobayas . Thanks!

            I can understand that the right usage is to use Double or Long explicitly ("100d" or "100L"). But random results don't seem to be good.

            Toshiya Kobayashi added a comment - I can understand that the right usage is to use Double or Long explicitly ("100d" or "100L"). But random results don't seem to be good.

              mfusco@redhat.com Mario Fusco
              rhn-support-tkobayas Toshiya Kobayashi
              Tibor Zimányi Tibor Zimányi
              Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: