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.

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

Change ParserBase to track floats independently from doubles #755

Closed
pjfanning opened this issue Apr 22, 2022 · 6 comments
Closed

Change ParserBase to track floats independently from doubles #755

pjfanning opened this issue Apr 22, 2022 · 6 comments

Comments

@pjfanning
Copy link
Member

pjfanning commented Apr 22, 2022

It seems like in some cases, casting a double to a float may result in a slightly different answer than parsing the original number string using Float.parseFloat.

This code in ParserBase (jackson-core) may be contributing to an issue that @plokhotnyuk described with parsing 1.199999988079071 as a float.

    @Override
    public float getFloatValue() throws IOException
    {
        double value = getDoubleValue();
        /* 22-Jan-2009, tatu: Bounds/range checks would be tricky
         *   here, so let's not bother even trying...
         */
        /*
        if (value < -Float.MAX_VALUE || value > MAX_FLOAT_D) {
            _reportError("Numeric value ("+getText()+") out of range of Java float");
        }
        */
        return (float) value;
    }

From my testing, it seems that casting doubles to floats may not get the same result as parsing the text representation directly with Float.parseFloat(String).

@pjfanning
Copy link
Member Author

pjfanning commented Apr 22, 2022

@plokhotnyuk could you check the test case in FasterXML/jackson-databind#3464 - result seems correct as is

@cowtowncoder
Copy link
Member

Hmmh. Yes, I agree.

Unfortunately this may also require more changes for cases where coercion is used (at least for textual formats) -- in case where double is requested first, decoded double will be used as the base to coerce from (due to assumption that coercion is faster than decoding).

I think what would be really useful is/are test cases to trigger the problem.

One quick question: was the specific textual representation 1.2? If so, could just verify that the float we get is same as Float.parseFloat("1.2"), for example.

@pjfanning
Copy link
Member Author

@cowtowncoder I still haven't reproduced the problem - so for now, this can be ignored. When I get a chance, I'll try the jsoniter-scala benchmark scenario directly to see if I can debug it there.

@pjfanning
Copy link
Member Author

@cowtowncoder I added FasterXML/jackson-databind#3470 - it shows the issue that @plokhotnyuk reported. Would you be able to take a look?

@pjfanning
Copy link
Member Author

@cowtowncoder #757 seems to fix the issue in FasterXML/jackson-databind#3470 -- could you review that PR?

@cowtowncoder
Copy link
Member

I think this was indeed resolved via #757, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants