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

ARROW-11706: [JS] Better BigInt compatibility check #9110

Closed
wants to merge 1 commit into from

Conversation

marcprux
Copy link
Member

@marcprux marcprux commented Jan 6, 2021

Check for whether BigInt64ArrayAvailable and BigUint64ArrayAvailable are available, rather than just BigIntAvailable. Recent versions of JavaScriptCore/WebKit in Safari support BigInt but do not support BigInt64Array, and so anything that relies on BigInt64Array will fail despite BigIntAvailable being true.

Check for whether `BigInt64ArrayAvailable` and `BigUint64ArrayAvailable` are available, rather than just `BigIntAvailable`. Recent versions of JavaScriptCore/WebKit in Safari support `BigInt` but do not support `BigInt64Array`, and so anything that relies on `BigInt64Array` will fail despite `BigIntAvailable` being `true`.
@github-actions
Copy link

github-actions bot commented Jan 6, 2021

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@marcprux
Copy link
Member Author

marcprux commented Jan 6, 2021

The manifestation of this issue can be seen when trying to run the following within Safari on a table that contains bigints:

RecordBatchJSONWriter.writeAll(table).toString(true)
message: "BigUint64Array is not available in this environment"
  BigUint64ArrayUnavailableError
  BigUint64ArrayUnavailable
  bignumToString
  bigNumsToStrings
  generatorResume@[native code]
  performIteration@[native code]
  visitInt
  visit
  map@[native code]
  recordBatchToJSON
  close
  finish
  global code

@marcprux
Copy link
Member Author

marcprux commented Jan 6, 2021

@dianaclarke
Copy link
Contributor

Thanks for the pull request!

I've created a Jira ticket for you:
https://issues.apache.org/jira/browse/ARROW-11706

If you could update the PR subject to ARROW-11706: [JS] Better BigInt compatibility check, you might have better luck catching the eye of JS Arrow reviewers (which I am not).

Thanks again & stay safe!

@marcprux marcprux changed the title Better BigInt compatibility check ARROW-11706: [JS] Better BigInt compatibility check Feb 20, 2021
@github-actions
Copy link

@kou
Copy link
Member

kou commented Apr 8, 2021

@trxcllnt Can we merge this?

@marcprux Could you tell us your JIRA account? I want to assign you to https://issues.apache.org/jira/browse/ARROW-11706 .

@trxcllnt
Copy link
Contributor

trxcllnt commented Apr 8, 2021

@kou Yeah, it looks good to me 👍

@marcprux we strongly recommend against using the JSON format outside the Arrow CI integration tests, and may remove the JSON IPC classes from the npm packages soon as part of slimming down the ArrowJS bundle sizes.

@domoritz
Copy link
Member

@trxcllnt said that instead of this change, we should remove the JSON IPC classes from the npm packages and only have them in the integration tests. Can we close this pull request?

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

Successfully merging this pull request may close these issues.

None yet

5 participants