You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "purplefox (via GitHub)" <gi...@apache.org> on 2023/04/24 19:37:44 UTC

[GitHub] [arrow] purplefox opened a new issue, #35310: Incorrect value when creating decimal128 from string (golang)

purplefox opened a new issue, #35310:
URL: https://github.com/apache/arrow/issues/35310

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   Hi All,
   
   I came across this behaviour when creating a decimal128 from a string:
   
   ```
   func TestDecFromStringStrangeness(t *testing.T) {
   	num, err := decimal128.FromString("2112.33", 38, 2)
   	require.NoError(t, err)
   	require.Equal(t, decimal128.New(0, 211233), num)
   }
   ```
   The value created seems to be out by one on the smallest digit:
   ```
   === RUN   TestDecFromStringBugMaybe
       decimal_test.go:54: 
           	Error Trace:	/Users/tfox/Development/tektite/types/decimal_test.go:54
           	Error:      	Not equal: 
           	            	expected: decimal128.Num{lo:0x33921, hi:0}
           	            	actual  : decimal128.Num{lo:0x33920, hi:0}
           	            	
           	            	Diff:
           	            	--- Expected
           	            	+++ Actual
           	            	@@ -1,3 +1,3 @@
           	            	 (decimal128.Num) {
           	            	- lo: (uint64) 211233,
           	            	+ lo: (uint64) 211232,
           	            	  hi: (int64) 0
           	Test:       	TestDecFromStringBugMaybe
   --- FAIL: TestDecFromStringBugMaybe (0.00s)
   
   ```
   
   ### Component(s)
   
   Go


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] zeroshade closed issue #35310: [Go] Incorrect value when creating decimal128 from string

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade closed issue #35310: [Go] Incorrect value when creating decimal128 from string
URL: https://github.com/apache/arrow/issues/35310


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] zeroshade commented on issue #35310: [Go] Incorrect value when creating decimal128 from string

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #35310:
URL: https://github.com/apache/arrow/issues/35310#issuecomment-1521930259

   @purplefox We do parse the decimal directly. The issue is that we need to convert the parsed value to an integer by multiplying by 10^(scale). ie: 2112.33 -> 2112.33 * 10^2 ->  211233 (internal representation in decimal128). In addition, sometimes we *want* the rounding to occur based on the passed in precision/scale. If you parse a string value but set the precision low enough, it will round appropriately.
   
   Unless you mean writing the code to parse the string itself without using `big.ParseFloat`, and that's certainly an option but I wouldn't call it simpler as there are a lot of edge cases that would have to be covered which we get for free by using `big.ParseFloat` such as handling Infinity, and NaN etc. You can see an example in the C++ code which does implement the parsing itself since they didn't have something like the `math/big` library that already existed.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] purplefox commented on issue #35310: [Go] Incorrect value when creating decimal128 from string

Posted by "purplefox (via GitHub)" <gi...@apache.org>.
purplefox commented on issue #35310:
URL: https://github.com/apache/arrow/issues/35310#issuecomment-1521167681

   Thinking... it might be simpler and safer to parse the decimal directly from the string without going via a float to guarantee no rounding errors?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] zeroshade commented on issue #35310: Incorrect value when creating decimal128 from string (golang)

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #35310:
URL: https://github.com/apache/arrow/issues/35310#issuecomment-1520806180

   Well this is just interesting. If I use precision <= 34 with the same value, then I get the expected `211233`. But if you use `35 <= prec <= 38` then you end up with this rounding issue. I'm not quite sure why it's happening yet, though it's likely something to do with the `SetPrec` call in the `FromString` function along with attempting to represent this value as a float. I'll see if i can try to figure out a way to fix this without breaking other cases.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] zeroshade commented on issue #35310: [Go] Incorrect value when creating decimal128 from string

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #35310:
URL: https://github.com/apache/arrow/issues/35310#issuecomment-1520808195

   Also just adding on that this doesn't replicate as a problem if using `decimal256` instead of `decimal128`. `decimal256` with a precision of `38` parses this value correctly without rounding.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org