Fix insert of BigDecimal with config - #695
Conversation
WalkthroughAdds a NUMERIC(...) special-case in Table.toSqlParams for Custom types so array types emit NUMERIC(p, s)[] directly; updates GraphQL test schema, event handlers, and SerDe/SQL tests to add a BigDecimal field with precision/scale and assert NUMERIC(10, 8) typing in DDL and unnest paths. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Schema as GraphQL Schema
participant Codegen as Code Generator
participant Table as Table.toSqlParams
participant Tests as SQL Tests
Schema->>Codegen: Add BigDecimal field with @config(precision:10, scale:8)
Codegen->>Table: toSqlParams(fieldType="NUMERIC(10, 8)", isArray=true)
alt fieldType startsWith "NUMERIC("
Table-->>Codegen: return "NUMERIC(10, 8)[]"
else
Table-->>Codegen: return previous array typing (text[] cast path)
end
Codegen->>Tests: Emit DDL and UNNEST bindings using NUMERIC(10, 8) / NUMERIC(10, 8)[]
Tests-->>Codegen: Assert expected DDL / unnest parameter types
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
codegenerator/cli/npm/envio/src/db/Table.res (1)
241-247: Fix is correct: avoid schema-qualifying parameterized NUMERIC typesHandling Custom(fieldType) when it starts with "NUMERIC(" to produce NUMERIC(...)[] prevents generating invalid casts like TEXT[]::"schema".NUMERIC(10, 8) and matches the tests’ expectation ($n::NUMERIC(10, 8)[]). Boolean and enum branches remain intact.
To make this more robust and future-proof:
- Make the check case-insensitive and accept DECIMAL(…) as a synonym.
- Trim leading spaces before the check to avoid issues if formatting changes upstream.
Proposed change within this switch (keeps behavior identical for NUMERIC, but expands support):
- | Custom(fieldType) if fieldType->Js.String2.startsWith("NUMERIC(") => fieldType + | Custom(fieldType) if fieldType->isNumericParamType => fieldTypeAdd this helper near the top of the file (outside the selected range):
let isNumericParamType = (t: string) => { let u = t->Js.String2.trim->Js.String2.toUpperCase u->Js.String2.startsWith("NUMERIC(") || u->Js.String2.startsWith("DECIMAL(") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
codegenerator/cli/npm/envio/src/db/Table.res(1 hunks)scenarios/test_codegen/schema.graphql(2 hunks)scenarios/test_codegen/src/EventHandlers.ts(2 hunks)scenarios/test_codegen/test/SerDe_Test.res(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
scenarios/test_codegen/**/schema.graphql
📄 CodeRabbit Inference Engine (scenarios/test_codegen/.cursor/rules/hyperindex.mdc)
scenarios/test_codegen/**/schema.graphql: After changing schema.graphql, run: pnpm codegen
Do not add the @entity decorator to GraphQL schema types
Avoid time-series aggregate fields (e.g., dailyVolume) in the GraphQL schema
Never use arrays of entities in the schema (e.g., [User!]!)
Model relationships using *_id fields in the schema (e.g., user_id: String!) instead of entity references
Files:
scenarios/test_codegen/schema.graphql
scenarios/test_codegen/**/*.ts
📄 CodeRabbit Inference Engine (scenarios/test_codegen/.cursor/rules/hyperindex.mdc)
scenarios/test_codegen/**/*.ts: After changing any TypeScript files, run: pnpm tsc --noEmit to ensure successful compilation
When updating existing entities in handlers, always use the spread operator to create updated objects before persisting
For any external call (e.g., fetch), wrap it in an Effect via experimental_createEffect and consume via context.effect
Use !context.isPreload to skip logic that should not run during preload
In TypeScript, set relationship fields using *_id properties (e.g., token_id) rather than object references
Always cast timestamps from events to BigInt (e.g., BigInt(event.block.timestamp))
When matching addresses in configuration objects within code, ensure keys are lowercase and compare using address.toLowerCase()
Use string | undefined for optional string fields instead of string | null
Always normalize token amounts to a standard decimal (e.g., 18) before addition across tokens; use helpers like normalizeAmountToUSD()
Files:
scenarios/test_codegen/src/EventHandlers.ts
**/*.{res,resi}
📄 CodeRabbit Inference Engine (.cursor/rules/rescript.mdc)
**/*.{res,resi}: Never use[| item |]to create an array. Use[ item ]instead.
Must always use=for setting value to a field. Use:=only for ref values created usingreffunction.
ReScript has record types which require a type definition before hand. You can access record fields by dot likefoo.myField.
It's also possible to define an inline object, it'll have quoted fields in this case.
Use records when working with structured data, and objects to conveniently pass payload data between functions.
Never use %raw to access object fields if you know the type.
Files:
codegenerator/cli/npm/envio/src/db/Table.resscenarios/test_codegen/test/SerDe_Test.res
codegenerator/cli/npm/envio/**
📄 CodeRabbit Inference Engine (.cursor/rules/navigation.mdc)
Library-fied runtime shared across indexers lives in codegenerator/cli/npm/envio.
Files:
codegenerator/cli/npm/envio/src/db/Table.res
🧠 Learnings (4)
📚 Learning: 2025-08-11T08:49:57.399Z
Learnt from: CR
PR: enviodev/hyperindex#0
File: scenarios/test_codegen/.cursor/rules/hyperindex.mdc:0-0
Timestamp: 2025-08-11T08:49:57.399Z
Learning: Applies to scenarios/test_codegen/**/schema.graphql : Do not add the entity decorator to GraphQL schema types
Applied to files:
scenarios/test_codegen/schema.graphql
📚 Learning: 2025-08-11T08:49:57.399Z
Learnt from: CR
PR: enviodev/hyperindex#0
File: scenarios/test_codegen/.cursor/rules/hyperindex.mdc:0-0
Timestamp: 2025-08-11T08:49:57.399Z
Learning: Applies to scenarios/test_codegen/**/schema.graphql : Avoid time-series aggregate fields (e.g., dailyVolume) in the GraphQL schema
Applied to files:
scenarios/test_codegen/schema.graphql
📚 Learning: 2025-08-11T08:49:57.399Z
Learnt from: CR
PR: enviodev/hyperindex#0
File: scenarios/test_codegen/.cursor/rules/hyperindex.mdc:0-0
Timestamp: 2025-08-11T08:49:57.399Z
Learning: Applies to scenarios/test_codegen/**/*.ts : When updating existing entities in handlers, always use the spread operator to create updated objects before persisting
Applied to files:
scenarios/test_codegen/src/EventHandlers.ts
📚 Learning: 2025-08-11T08:49:57.399Z
Learnt from: CR
PR: enviodev/hyperindex#0
File: scenarios/test_codegen/.cursor/rules/hyperindex.mdc:0-0
Timestamp: 2025-08-11T08:49:57.399Z
Learning: Applies to scenarios/test_codegen/**/*.ts : Always cast timestamps from events to BigInt (e.g., BigInt(event.block.timestamp))
Applied to files:
scenarios/test_codegen/src/EventHandlers.ts
🔇 Additional comments (6)
scenarios/test_codegen/src/EventHandlers.ts (1)
543-543: LGTM: new non-null BigDecimal field is set in both entitiesBoth inserts now populate bigDecimalWithConfig, aligning with the schema change making it non-nullable. No logic side-effects.
Per repo guidelines for scenarios/test_codegen:
- After TS changes: run pnpm tsc --noEmit
- After schema changes: run pnpm codegen
Also applies to: 567-567
scenarios/test_codegen/schema.graphql (1)
117-117: Schema addition looks correct; remember to regenerate typesbigDecimalWithConfig: BigDecimal! @config(precision: 10, scale: 8) is correctly added to both types. This will drive NUMERIC(10, 8) in SQL and affects generated TS types.
Run pnpm codegen to update generated types used in handlers/tests.
Also applies to: 145-145
scenarios/test_codegen/test/SerDe_Test.res (4)
37-38: LGTM: entity test data includes configured BigDecimalAdding bigDecimalWithConfig to the all-types entity mirrors the schema and ensures SerDe coverage.
127-127: LGTM: CREATE TABLE expectation includes NUMERIC(10, 8) columnThe expected DDL for EntityWithAllNonArrayTypes now includes "bigDecimalWithConfig" NUMERIC(10, 8) NOT NULL, consistent with the schema directive.
138-140: LGTM: UNNEST query casts use NUMERIC(10, 8)[]The INSERT … SELECT FROM unnest expectation correctly uses $2::NUMERIC(10, 8)[], validating the generator change in Table.res.
160-161: LGTM: non-array entity test data includes configured BigDecimalThis ensures the non-array code path and history serialization also cover the new field.
| switch field { | ||
| | Field(f) => | ||
| switch f.fieldType { | ||
| | Custom(fieldType) if fieldType->Js.String2.startsWith("NUMERIC(") => fieldType |
There was a problem hiding this comment.
Can you just add a comment above this when it would start with Numeric? It's obviously an edge case
2 participants
Summary by CodeRabbit
New Features
Improvements
Tests