Skip to content

Add GremlinLang number type suffixes and fix Long deserialization in JS GLV#3340

Open
kirill-stepanishin wants to merge 3 commits intoapache:masterfrom
kirill-stepanishin:js-number-handling
Open

Add GremlinLang number type suffixes and fix Long deserialization in JS GLV#3340
kirill-stepanishin wants to merge 3 commits intoapache:masterfrom
kirill-stepanishin:js-number-handling

Conversation

@kirill-stepanishin
Copy link
Contributor

Adds proper Gremlin type suffixes to GremlinLang string generation and fixes precision loss in Long deserialization.

GremlinLang (gremlin-lang.ts)

  • Integers in Int32 range → bare (42), beyond Int32 → L suffix (3000000000L)
  • Floats → D suffix (3.14D)
  • bigintN suffix (5N), previously unhandled
  • Large integers in scientific notation fall back to D

Long deserialization (LongSerializer.js)

  • Values outside MIN_SAFE_INTEGER-MAX_SAFE_INTEGER now return BigInt instead of a precision-losing Number via parseFloat()

NumberSerializationStrategy fix

  • Replaced broken Int64 range check (used Number literals beyond safe range, silently rounded) with correct MIN_SAFE_INTEGER..MAX_SAFE_INTEGER bounds

Test updates

  • Updated 3 existing GremlinLang expectations to reflect D suffix on floats
  • Added tests for bigint, Int32 boundaries, safe integer boundaries, and float suffix
  • max-long/min-long model values changed from imprecise Number to exact BigInt
  • min-long moved from byte-exact (run) to object-level (runWriteRead) — previously passed only because precision loss was symmetrical in both directions

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.49%. Comparing base (cfd6889) to head (8224ea4).
⚠️ Report is 912 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3340      +/-   ##
============================================
- Coverage     77.87%   75.49%   -2.38%     
+ Complexity    13578    13309     -269     
============================================
  Files          1015     1023       +8     
  Lines         59308    60252     +944     
  Branches       6835     7075     +240     
============================================
- Hits          46184    45487     -697     
- Misses        10817    12079    +1262     
- Partials       2307     2686     +379     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kenhuuu
Copy link
Contributor

kenhuuu commented Mar 25, 2026

Question: if a user inputs a number that is larger than the max long, it isn't suffixed to be a BigInteger?

@kirill-stepanishin
Copy link
Contributor Author

Question: if a user inputs a number that is larger than the max long, it isn't suffixed to be a BigInteger?

Correct. A user can't even input such a value accurately as a number in the first place, since JS number only has 53 bits of precision vs 63 bits for Java Long. Any integer-looking number beyond safe range has already lost precision, so it gets D. For true BigInteger, users need to use JS bigint type.

@Cole-Greer
Copy link
Contributor

Thanks @kirill-stepanishin, the changes look great

VOTE +1

@kenhuuu
Copy link
Contributor

kenhuuu commented Mar 25, 2026

VOTE +1

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants