-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-5714: [JS] Inconsistent behavior in Int64Builder with/without BigNum #4691
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
ARROW-5714: [JS] Inconsistent behavior in Int64Builder with/without BigNum #4691
Conversation
|
@TheNeuralBit I was thinking about this more last night, and while this works for the JS-number case, it won't work if we try to set in a pair of |
|
@TheNeuralBit TheNeuralBit#3 here's a pull that should do what we want in FF |
Codecov Report
@@ Coverage Diff @@
## master #4691 +/- ##
=========================================
+ Coverage 88.94% 90.2% +1.25%
=========================================
Files 705 101 -604
Lines 81275 6399 -74876
Branches 1420 1420
=========================================
- Hits 72288 5772 -66516
+ Misses 8977 617 -8360
Partials 10 10Continue to review full report at Codecov.
|
3688820 to
2e4ce7a
Compare
Use WideBufferBuilder for Int64 and Uint64 builders
* Also modifies logic slightly, now the various input types are evenly distributed.
|
@trxcllnt I merged your PR and also clarified the int64 generation for the builder tests. Does this look ok to you? |
|
@TheNeuralBit yep, that looks great. thanks! |
DataBufferBuilder.setto ensure that Int64Builder is consistent with and without BigNum.WideBufferBuilderforInt64andUint64builders, even whenBigIntis not available. Moves check forBigIntavailability intoWideBufferBuilder. (Thanks to Paul Taylor)