fix: handle Arrow Int64 BigInt values in accessors and filters - #3641
Open
igorDykhta wants to merge 2 commits into
Open
fix: handle Arrow Int64 BigInt values in accessors and filters#3641igorDykhta wants to merge 2 commits into
igorDykhta wants to merge 2 commits into
Conversation
added 2 commits
August 16, 2026 07:59
Signed-off-by: Ihor Dykhta <ihordykhta@Ihors-MacBook-Pro.local>
Signed-off-by: Ihor Dykhta <ihordykhta@Ihors-MacBook-Pro.local>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves handling of Arrow Int64/Uint64 columns (which materialize as JS bigint) by routing reads through field valueAccessors (which can coerce Int64 to number), and by updating formatting/aggregation/filtering paths to avoid d3-format failures on bigint.
Changes:
- Add Int64-aware
valueAccessorbinding inKeplerTable(convert rawbigintcell values tonumberfor consumers). - Update aggregation/plotting, filter plotting, GPU filter accessors, and several layers to use
field.valueAccessor(...)instead ofdataContainer.valueAt(...). - Add node tests covering Int64 accessors and BigInt formatting.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/node/utils/kepler-table-test.js | Adds coverage for Int64 Arrow columns: raw valueAt stays bigint, valueAccessor yields number. |
| test/node/utils/data-utils-test.js | Adds coverage ensuring numeric formatters can handle bigint inputs. |
| src/utils/src/plot.ts | Routes aggregation accessors through valueAccessor (esp. percent fields). |
| src/utils/src/filter-utils.ts | Uses valueAccessor when building y-series for non-histogram plots. |
| src/utils/src/data-utils.ts | Switches numeric tooltip formatting to a BigInt-safe wrapper. |
| src/table/src/kepler-table.ts | Introduces Int64 Arrow column detection + valueAccessor conversion to number. |
| src/table/src/gpu-filter-utils.ts | Makes GPU filter accessor prefer field.valueAccessor (Int64-safe) with optional custom getData. |
| src/layers/src/trip-layer/trip-layer.ts | Uses valueAccessor for GeoJSON column-mode value reads. |
| src/layers/src/heatmap-layer/heatmap-layer.ts | Simplifies GPU filter accessor wiring to rely on valueAccessor. |
| src/layers/src/geojson-layer/geojson-layer.ts | Uses dataset fields + valueAccessor for filter values (incl. text labels). |
| src/layers/src/flow-layer/flow-layer.ts | Uses valueAccessor for magnitudes and coerces tooltip formatting for BigInt. |
| src/layers/src/aggregation-layer.ts | Uses field.valueAccessor in aggregation and GPU filtering paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+457
to
+461
| /** d3-format cannot take BigInt; Int64 table cells are still raw bigint from valueAt. */ | ||
| function formatNumeric(spec: string): FieldFormatter { | ||
| const format = d3Format(spec); | ||
| return v => format(Number(v)); | ||
| } |
| fieldValues: Array<{labelMessage: string; value: any}>; | ||
| } | null { | ||
| const fmt = d3Format(TOOLTIP_FORMATS.DECIMAL_COMMA.format); | ||
| const fmt = v => d3Format(TOOLTIP_FORMATS.DECIMAL_COMMA.format)(Number(v)); |
Comment on lines
384
to
387
| return { | ||
| getNumerator: i => dataContainer.valueAt(i, numeratorIdx), | ||
| getDenominator: i => dataContainer.valueAt(i, denominatorIdx) | ||
| getNumerator: i => fields[numeratorIdx].valueAccessor({index: i}), | ||
| getDenominator: i => fields[denominatorIdx].valueAccessor({index: i}) | ||
| }; |
Comment on lines
+162
to
+164
| const datum = d || objectInfo; | ||
| // field.valueAccessor is Number() for Int64 columns; valueAt stays raw BigInt. | ||
| const data = getData ? getData(dc, datum, fieldIndex) : field.valueAccessor(datum); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Parquet/Arrow Int64 columns surface as JS
BigInt, which breaks d3 scales, GPU filters (bigint - number), and integer formatters.Number()onfield.valueAccessoronce per Int64 column at import; other columns are unchanged.valueAtraw so H3/A5 64-bit ids stay exact.valueAccessor.Test plan