feat: Add support for Decimal128 data type#10083
feat: Add support for Decimal128 data type#10083YazanAlkhalil wants to merge 8 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! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
✅ 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Decimal128 support end-to-end: tests, Mongo and Postgres adapters, schema validation, GraphQL types, and Mongo↔REST 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 Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (5 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.11% 92.54% +0.43%
==========================================
Files 192 192
Lines 16566 16641 +75
Branches 231 231
==========================================
+ Hits 15259 15401 +142
+ Misses 1281 1218 -63
+ Partials 26 22 -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.
PR Status UpdateAll Decimal128-related code and tests are passing. The only remaining CI failure is a pre-existing flaky test unrelated to this PR:
This same test is also failing on the All other checks pass, including:
This PR is ready for review. Could a maintainer please re-run the failed Redis Cache job and review when convenient? |
There was a problem hiding this comment.
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)
92-113:⚠️ Potential issue | 🟠 MajorFinish the dot-notation Decimal128 query path.
Top-level Decimal128 columns look fine, but dotted/nested queries still have two gaps: the comparator path treats Decimal128 as text instead of
numeric, and the directfieldValue.__type === 'Decimal128'branch still appends a:namepredicate for dotted keys. That can turn nested equality queries into false negatives and nested range queries into lexicographic comparisons.💡 Suggested fix
const toPostgresValueCastType = value => { + if (value && typeof value === 'object' && value.__type === 'Decimal128') { + return 'numeric'; + } const postgresValue = toPostgresValue(value); let castType; switch (typeof postgresValue) { @@ if (fieldName.indexOf('.') >= 0) { const castType = toPostgresValueCastType(fieldValue.$eq); const constraintFieldName = castType ? `CAST ((${transformDotField(fieldName)}) AS ${castType})` : transformDotField(fieldName); - values.push(fieldValue.$eq); + values.push(toPostgresValue(fieldValue.$eq)); patterns.push(`${constraintFieldName} = $${index++}`); @@ - if (fieldValue.__type === 'Decimal128') { + if (fieldName.indexOf('.') < 0 && fieldValue.__type === 'Decimal128') { patterns.push(`$${index}:name = $${index + 1}`); values.push(fieldName, fieldValue.value); index += 2; }Also applies to: 452-472, 844-848, 850-886
🤖 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 92 - 113, The dotted/nested Decimal128 path is being treated as text and still appends a :name predicate, causing wrong comparisons; update the conversion and comparator logic so that toPostgresValueCastType returns 'numeric' when the resolved postgresValue represents a Decimal128 (i.e., when toPostgresValue(value) yields an object with __type === 'Decimal128' or when the comparator path resolves a nested field whose value.__type === 'Decimal128'), and change the comparator generation code that builds predicates for dotted keys to avoid adding the extra ":name" text-suffix for Decimal128 nested fields (use a numeric parameter/cast instead of a :name text predicate). Apply the same fix to the other similar blocks noted (lines ~452-472, ~844-848, ~850-886) so nested equality and range queries use numeric casting and numeric parameters for Decimal128 values rather than string/text comparisons.
🧹 Nitpick comments (1)
src/Controllers/SchemaController.js (1)
503-503: Please confirm the public docs mentionDecimal128.This makes
Decimal128a first-class Parse schema type, but I don't see accompanying docs in the provided diff. A short README / API note for the REST wire format and GraphQL scalar usage would make the feature much easier to discover.Based on learnings, when Parse Server adds a new feature it's worth checking whether that feature is documented in
README.md, even if the doc follow-up lands separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Controllers/SchemaController.js` at line 503, The review flags that adding 'Decimal128' as a first-class Parse schema type (the literal 'Decimal128' in SchemaController.js) needs corresponding public docs; update the public documentation (README.md and any REST API and GraphQL scalar docs) to describe the Decimal128 type, include the REST wire format/example payloads, how it appears in schema responses, and how it maps to/should be used as a GraphQL scalar (including examples and any serialization/deserialization rules); reference the SchemaController.js symbol 'Decimal128' in the docs so consumers can discover the new type and its usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js`:
- Around line 92-113: The dotted/nested Decimal128 path is being treated as text
and still appends a :name predicate, causing wrong comparisons; update the
conversion and comparator logic so that toPostgresValueCastType returns
'numeric' when the resolved postgresValue represents a Decimal128 (i.e., when
toPostgresValue(value) yields an object with __type === 'Decimal128' or when the
comparator path resolves a nested field whose value.__type === 'Decimal128'),
and change the comparator generation code that builds predicates for dotted keys
to avoid adding the extra ":name" text-suffix for Decimal128 nested fields (use
a numeric parameter/cast instead of a :name text predicate). Apply the same fix
to the other similar blocks noted (lines ~452-472, ~844-848, ~850-886) so nested
equality and range queries use numeric casting and numeric parameters for
Decimal128 values rather than string/text comparisons.
---
Nitpick comments:
In `@src/Controllers/SchemaController.js`:
- Line 503: The review flags that adding 'Decimal128' as a first-class Parse
schema type (the literal 'Decimal128' in SchemaController.js) needs
corresponding public docs; update the public documentation (README.md and any
REST API and GraphQL scalar docs) to describe the Decimal128 type, include the
REST wire format/example payloads, how it appears in schema responses, and how
it maps to/should be used as a GraphQL scalar (including examples and any
serialization/deserialization rules); reference the SchemaController.js symbol
'Decimal128' in the docs so consumers can discover the new type and its usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed210c9f-d429-46e2-b7e4-e350a34142ed
📒 Files selected for processing (4)
src/Adapters/Storage/Mongo/MongoTransform.jssrc/Adapters/Storage/Postgres/PostgresStorageAdapter.jssrc/Controllers/SchemaController.jssrc/GraphQL/loaders/defaultGraphQLTypes.js
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