feat: Add support for Decimal128 data type#10083
feat: Add support for Decimal128 data type#10083YazanAlkhalil wants to merge 6 commits intoparse-community:alphafrom
Conversation
Adds Decimal128 support using `{ __type: 'Decimal128', value: '<string>' }` REST format, enabling high-precision decimal numbers for monetary values, blockchain amounts, and other use cases that exceed the precision of the Number (double) type.
Changes:
- Schema: Register Decimal128 as a valid field type
- MongoDB: Store as native Decimal128, with full coder for JSON/DB conversion
- PostgreSQL: Map to `numeric` type for equivalent precision
- GraphQL: Add Decimal128 scalar type and Decimal128WhereInput filter
- Tests: 10 tests covering CRUD, comparison queries, and edge cases
Closes parse-community#8840
|
🚀 Thanks for opening this pull request! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Decimal128 support across tests, Mongo and Postgres adapters, schema validation, GraphQL types, and transformation logic to persist, query, and round-trip Decimal128 values. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ParseServer
participant SchemaController
participant MongoDB
participant Postgres
rect rgba(0, 128, 255, 0.5)
Client->>ParseServer: Send REST/GraphQL request with Decimal128 payload
ParseServer->>SchemaController: Validate/resolve field type (Decimal128)
end
alt storage = MongoDB
ParseServer->>MongoDB: Convert JSON __type -> DB Decimal128 (Decimal128Coder)
MongoDB-->>ParseServer: Return DB Decimal128 value
ParseServer->>Client: Convert DB Decimal128 -> JSON __type response
else storage = Postgres
ParseServer->>Postgres: Send numeric string for Decimal128
Postgres-->>ParseServer: Return numeric value/string
ParseServer->>Client: Wrap numeric as JSON __type Decimal128
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10083 +/- ##
==========================================
+ Coverage 92.19% 92.63% +0.44%
==========================================
Files 191 191
Lines 15762 15837 +75
Branches 176 176
==========================================
+ Hits 14531 14670 +139
+ Misses 1215 1155 -60
+ Partials 16 12 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
spec/ParseDecimal128.spec.js (1)
10-14: Consider extracting shared headers to reduce duplication.Same Parse auth and JSON headers are repeated throughout the file; a small constant/helper would make this spec easier to maintain.
Also applies to: 42-45, 89-93, 124-127, 157-160, 182-185, 209-212, 236-239, 263-266, 300-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/ParseDecimal128.spec.js` around lines 10 - 14, Extract the repeated Parse headers into a single reusable constant or helper at the top of spec/ParseDecimal128.spec.js (e.g., const PARSE_HEADERS or function getParseHeaders()) and replace the duplicated inline headers in all HTTP requests (the objects currently passed where 'X-Parse-Application-Id', 'X-Parse-Master-Key', and 'Content-Type' are set) with that constant/helper so tests reuse the same header set and reduce duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/ParseDecimal128.spec.js`:
- Around line 151-175: The test currently only asserts the returned Decimal128
has a value, which doesn’t ensure precision is preserved; replace the loose
check on getResponse.data.amount.value with a strict equality assertion against
the original highPrecisionValue string (keep the existing assert for __type ===
'Decimal128'), i.e. assert getResponse.data.amount.value === highPrecisionValue
using the same variable highPrecisionValue so the test fails if digits were
truncated/rounded after the createResponse/getResponse round-trip.
In `@src/Adapters/Storage/Mongo/MongoTransform.js`:
- Around line 1481-1487: The isValidJSON implementation is too permissive:
update isValidJSON to verify the input is an object with __type === 'Decimal128'
and that it has a present string 'value' property (typeof value === 'string')
and optionally that it matches a Decimal128-safe numeric/string pattern; also
update JSONToDatabase (function JSONToDatabase) to defensively handle or throw
if json.value is missing or not a string before calling
mongodb.Decimal128.fromString, ensuring malformed payloads are rejected early
rather than causing downstream failures.
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js`:
- Around line 805-809: createConstraint currently pushes raw list items for
IN/NIN filters, so Decimal128 entries remain objects and break Postgres
comparisons; update the list-handling branch inside createConstraint to detect
items where item.__type === 'Decimal128' and, instead of pushing the object,
push the field name and the scalar string item.value into the values array while
emitting corresponding parameterized patterns (matching the existing Decimal128
equality logic that uses patterns.push(`$${index}:name = $${index + 1}`) and
increments index by 2). Ensure both $in and $nin branches convert Decimal128
list members to their .value and use the same parameter naming/placement logic
so all list parameters are scalars that Postgres can compare.
In `@src/Controllers/SchemaController.js`:
- Around line 1631-1634: The Decimal128 branch in SchemaController.js currently
accepts any non-null obj.value which allows numbers/objects through; update the
case 'Decimal128' check to only accept string payloads by verifying typeof
obj.value === 'string' (and non-empty if desired) before returning 'Decimal128'
so only string values are detected as Decimal128; locate the switch/case for
'Decimal128' in SchemaController.js and modify the condition around obj.value
accordingly.
In `@src/GraphQL/loaders/defaultGraphQLTypes.js`:
- Around line 320-329: The DECIMAL128.parseLiteral currently only accepts
Kind.STRING but must mirror parseValue by also accepting inline object literals;
update DECIMAL128.parseLiteral to handle both AST kinds: if ast.kind ===
Kind.STRING return { __type: 'Decimal128', value: ast.value }, and if ast.kind
=== Kind.OBJECT iterate ast.fields to extract the "__type" and "value" entries
(validate __type === 'Decimal128' and coerce value to a string), returning the
same shape as parseValue; if neither case matches, throw
TypeValidationError(ast.kind, 'Decimal128')—refer to the existing parseValue,
parseLiteral, DECIMAL128 symbol and TypeValidationError for implementation
details.
---
Nitpick comments:
In `@spec/ParseDecimal128.spec.js`:
- Around line 10-14: Extract the repeated Parse headers into a single reusable
constant or helper at the top of spec/ParseDecimal128.spec.js (e.g., const
PARSE_HEADERS or function getParseHeaders()) and replace the duplicated inline
headers in all HTTP requests (the objects currently passed where
'X-Parse-Application-Id', 'X-Parse-Master-Key', and 'Content-Type' are set) with
that constant/helper so tests reuse the same header set and reduce duplication.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
spec/ParseDecimal128.spec.jssrc/Adapters/Storage/Mongo/MongoSchemaCollection.jssrc/Adapters/Storage/Mongo/MongoTransform.jssrc/Adapters/Storage/Postgres/PostgresStorageAdapter.jssrc/Controllers/SchemaController.jssrc/GraphQL/loaders/defaultGraphQLTypes.jssrc/GraphQL/transformers/constraintType.jssrc/GraphQL/transformers/inputType.jssrc/GraphQL/transformers/outputType.js
| if (fieldValue.__type === 'Decimal128') { | ||
| patterns.push(`$${index}:name = $${index + 1}`); | ||
| values.push(fieldName, fieldValue.value); | ||
| index += 2; | ||
| } |
There was a problem hiding this comment.
Decimal128 support is incomplete for $in / $nin filters.
You added direct equality handling, but createConstraint() still pushes raw list items for IN/NOT IN. Decimal128 list entries remain objects instead of scalar numeric strings, which can break Postgres comparisons.
💡 Suggested fix (in `createConstraint` list handling)
- baseArray.forEach((listElem, listIndex) => {
+ baseArray.forEach((listElem, listIndex) => {
if (listElem != null) {
- values.push(listElem);
+ values.push(toPostgresValue(listElem));
inPatterns.push(`$${index + 1 + listIndex}`);
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js` around lines 805 -
809, createConstraint currently pushes raw list items for IN/NIN filters, so
Decimal128 entries remain objects and break Postgres comparisons; update the
list-handling branch inside createConstraint to detect items where item.__type
=== 'Decimal128' and, instead of pushing the object, push the field name and the
scalar string item.value into the values array while emitting corresponding
parameterized patterns (matching the existing Decimal128 equality logic that
uses patterns.push(`$${index}:name = $${index + 1}`) and increments index by 2).
Ensure both $in and $nin branches convert Decimal128 list members to their
.value and use the same parameter naming/placement logic so all list parameters
are scalars that Postgres can compare.
Adds tests for DECIMAL128 parseValue, serialize, and parseLiteral methods to improve code coverage on the new Decimal128 type.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/defaultGraphQLTypes.spec.js (1)
534-629: Consider adding edge-case test values for high-precision decimals.The tests use simple values like
'123.456'. Since Decimal128's purpose is high-precision storage (e.g., monetary/blockchain values), consider adding a test with a high-precision value to document this capability:it('should handle high-precision values', () => { expect(parseValue('12345678901234567890.12345678901234')).toEqual({ __type: 'Decimal128', value: '12345678901234567890.12345678901234', }); });This is optional since the GraphQL layer passes strings through without validation—actual precision validation occurs at the database adapter level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/defaultGraphQLTypes.spec.js` around lines 534 - 629, Tests for DECIMAL128 (parseLiteral, parseValue, serialize) only use low-precision example '123.456'; add an edge-case test to ensure high-precision decimal strings round-trip through the GraphQL scalars. Add a new spec in the Decimal128 describe block (near existing parseValue/serialize tests) that calls DECIMAL128.parseValue (and optionally parseLiteral/serialize) with a high-precision string such as '12345678901234567890.12345678901234' and asserts the returned object (or serialized value) preserves that exact string (__type: 'Decimal128', value: '<same-string>'). Ensure you reference DECIMAL128.parseValue, DECIMAL128.parseLiteral, and DECIMAL128.serialize when adding the assertions so the tests validate pass-through of high-precision strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/defaultGraphQLTypes.spec.js`:
- Around line 534-629: Tests for DECIMAL128 (parseLiteral, parseValue,
serialize) only use low-precision example '123.456'; add an edge-case test to
ensure high-precision decimal strings round-trip through the GraphQL scalars.
Add a new spec in the Decimal128 describe block (near existing
parseValue/serialize tests) that calls DECIMAL128.parseValue (and optionally
parseLiteral/serialize) with a high-precision string such as
'12345678901234567890.12345678901234' and asserts the returned object (or
serialized value) preserves that exact string (__type: 'Decimal128', value:
'<same-string>'). Ensure you reference DECIMAL128.parseValue,
DECIMAL128.parseLiteral, and DECIMAL128.serialize when adding the assertions so
the tests validate pass-through of high-precision strings.
- Tighten Decimal128 validation to require string values in SchemaController - Add string type check in MongoTransform Decimal128Coder isValidJSON/JSONToDatabase - Handle Decimal128 in Postgres $in/$nin by using toPostgresValue() - Add Kind.OBJECT support in GraphQL DECIMAL128 parseLiteral - Strengthen high-precision test to assert exact round-trip equality
Cover constraintType, inputType, and outputType transformer functions to improve patch coverage above the 92% threshold.
- Fix Decimal128 round-trip for values nested inside Object fields by handling deserialized BSON Decimal128 objects in isValidDatabaseObject and databaseToJSON - Use Decimal128Coder.isValidDatabaseObject() instead of raw instanceof in nestedMongoObjectToNestedParseObject and mongoObjectToParseObject - Add tests for nested Decimal128 and invalid value rejection
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (1)
492-501:⚠️ Potential issue | 🟠 Major
$in/$ninplaceholder indexing breaks with nullable list elements.At Line 497, placeholder numbering uses
listIndexfrom the original array while nulls are skipped. A query like$in: [null, {...}]can produce wrong bind positions (orIN ()), leading to invalid SQL at runtime.🔧 Proposed fix
const inPatterns = []; values.push(fieldName); - baseArray.forEach((listElem, listIndex) => { - if (listElem != null) { - values.push(toPostgresValue(listElem)); - inPatterns.push(`$${index + 1 + listIndex}`); - } - }); + const normalizedArray = baseArray.filter(listElem => listElem != null); + normalizedArray.forEach((listElem, listIndex) => { + values.push(toPostgresValue(listElem)); + inPatterns.push(`$${index + 1 + listIndex}`); + }); + if (inPatterns.length === 0) { + patterns.push(notIn ? '1 = 1' : `$${index}:name IS NULL`); + index += 1; + return; + } patterns.push(`$${index}:name ${not} IN (${inPatterns.join()})`); index = index + 1 + inPatterns.length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js` around lines 492 - 501, The placeholder math in PostgresStorageAdapter.js breaks when baseArray contains nulls because it uses listIndex (original array index) instead of counting only pushed values; update the loop in the block that builds inPatterns/values (the code using variables index, baseArray, inPatterns, values and toPostgresValue) to maintain a separate counter (e.g., inCount or cur) that increments only when you push a non-null element and use that counter to compute each placeholder (`$${...}`), ensure you join inPatterns with commas, and then advance index by 1 + inCount (not by inPatterns.length from the original listIndex mapping).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/GraphQL/loaders/defaultGraphQLTypes.js`:
- Around line 329-336: The parseLiteral path for DECIMAL128 is currently
allowing non-string AST literals because it checks typeof value.value.value ===
'string'; update DECIMAL128.parseLiteral to instead verify the AST node kind is
a STRING (use value.value.kind === Kind.STRING) so only GraphQL STRING literals
are accepted, matching parseValue behavior; locate the DECIMAL128.parseLiteral
implementation and replace the typeof check with a Kind.STRING check (ensure
Kind is imported/available in that module).
---
Outside diff comments:
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js`:
- Around line 492-501: The placeholder math in PostgresStorageAdapter.js breaks
when baseArray contains nulls because it uses listIndex (original array index)
instead of counting only pushed values; update the loop in the block that builds
inPatterns/values (the code using variables index, baseArray, inPatterns, values
and toPostgresValue) to maintain a separate counter (e.g., inCount or cur) that
increments only when you push a non-null element and use that counter to compute
each placeholder (`$${...}`), ensure you join inPatterns with commas, and then
advance index by 1 + inCount (not by inPatterns.length from the original
listIndex mapping).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
spec/ParseDecimal128.spec.jsspec/defaultGraphQLTypes.spec.jssrc/Adapters/Storage/Mongo/MongoTransform.jssrc/Adapters/Storage/Postgres/PostgresStorageAdapter.jssrc/Controllers/SchemaController.jssrc/GraphQL/loaders/defaultGraphQLTypes.js
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/defaultGraphQLTypes.spec.js
Prevents numeric AST literals (Kind.INT, Kind.FLOAT) from being accepted in the inline object syntax, matching parseValue behavior.
New Feature
Adds support for MongoDB's Decimal128 data type, enabling high-precision decimal numbers beyond the limits of the existing
Number(double) type.REST API Format
{ "__type": "Decimal128", "value": "12345678901234567890.123456789" }Motivation
Numbertype is limited to MongoDB's double precision, which loses precision for large numbersChanges
Decimal128as a valid field type with__typevalidationDecimal128type with full coder for JSON↔DB conversionnumerictype for equivalent high-precision supportDecimal128scalar type andDecimal128WhereInputfilter with all comparison operators$gt/$ltqueries, equality query, high-precision values, negative values, zero, schema API, and field deletionSDK Compatibility
No SDK changes are required. SDKs that don't natively support Decimal128 will receive and pass through the raw
{ __type: 'Decimal128', value: '...' }JSON object. SDK-specific helper classes (e.g.,Parse.Decimal128) can be added as follow-up enhancements.Closes #8840
Test plan
$gtand$ltcomparison operators__op: DeleteSummary by CodeRabbit
New Features
Tests