trxcllnt commented on code in PR #438:
URL: https://github.com/apache/arrow-js/pull/438#discussion_r3285980619
##########
src/visitor/vectorassembler.ts:
##########
@@ -234,11 +235,12 @@ function assembleBinaryViewVector<T extends BinaryView |
Utf8View>(this: VectorA
}
/** @ignore */
-function assembleListVector<T extends Map_ | List | FixedSizeList>(this:
VectorAssembler, data: Data<T>) {
+function assembleListVector<T extends Map_ | List | LargeList |
FixedSizeList>(this: VectorAssembler, data: Data<T>) {
const { length, valueOffsets } = data;
- // If we have valueOffsets (MapVector, ListVector), push that buffer first
+ // If we have valueOffsets (MapVector, ListVector, LargeListVector), push
that buffer first
if (valueOffsets) {
- const { [0]: begin, [length]: end } = valueOffsets;
+ const begin = bigIntToNumber(valueOffsets[0]);
+ const end = bigIntToNumber(valueOffsets[length]);
Review Comment:
Yes, if it was less work we would have already done it. Correctness is not
the place to compromise when there is a spec and an ecosystem of other
implementations we need to interoperate with.
I either wasn't involved or skimmed reviewing the LargeUtf8 and LargeBinary
PRs, but I would have said the same thing there. Luckily most of the helper
methods that need to be reimplemented in terms of `bigint` are straightforward
and can be reused for LargeUtf8/LargeBinary.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
