Repository navigation
Support non-nullable field additions in schema migrations via @default #3046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| type Transfer @entity { | ||
| id: ID! | ||
| amount: BigInt! | ||
| blockNumber: Int! | ||
| memo: String | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| type Transfer @entity { | ||
| id: ID! | ||
| amount: BigInt! | ||
| blockNumber: Int! | ||
| memo: String | ||
| fee: BigInt! @default(value: "0") | ||
| retries: Int! @default(value: "-1") | ||
| ratio: Float! @default(value: "0.5") | ||
| settled: Boolean! @default(value: "false") | ||
| label: String! @default(value: "it's") | ||
| status: TransferStatus! @default(value: "PENDING") | ||
| } | ||
|
|
||
| enum TransferStatus { | ||
| PENDING | ||
| DONE | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| type Transfer @entity { | ||
| id: ID! | ||
| amount: BigInt! | ||
| memo: String | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| type Transfer @entity { | ||
| id: ID! | ||
| amount: BigInt! | ||
| memo: String! @default(value: "none") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| type Transfer @entity { | ||
| id: ID! | ||
| amount: BigInt! | ||
| memo: String | ||
| fee: BigInt! | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -90,7 +90,12 @@ export function getAllEntitiesRelations(_schema: GraphQLSchema | string | null): | |||||||||
| ); | ||||||||||
|
|
||||||||||
| const modelRelations = {models: [], relations: [], enums: [...enums.values()]} as GraphQLModelsRelationsEnums; | ||||||||||
| const [derivedFrom, indexDirective, idDbType] = getDirectives(schema, ['derivedFrom', 'index', 'dbType']); | ||||||||||
| const [derivedFrom, indexDirective, idDbType, defaultDirective] = getDirectives(schema, [ | ||||||||||
| 'derivedFrom', | ||||||||||
| 'index', | ||||||||||
| 'dbType', | ||||||||||
| 'default', | ||||||||||
| ]); | ||||||||||
| for (const entity of entities) { | ||||||||||
| const newModel: GraphQLModelsType = { | ||||||||||
| name: entity.name, | ||||||||||
|
|
@@ -113,6 +118,8 @@ export function getAllEntitiesRelations(_schema: GraphQLSchema | string | null): | |||||||||
| const derivedFromDirectValues = field.astNode ? getDirectiveValues(derivedFrom, field.astNode) : undefined; | ||||||||||
| const indexDirectiveVal = field.astNode ? getDirectiveValues(indexDirective, field.astNode) : undefined; | ||||||||||
| const dbTypeDirectiveVal = field.astNode ? getDirectiveValues(idDbType, field.astNode) : undefined; | ||||||||||
| const defaultDirectiveVal = field.astNode ? getDirectiveValues(defaultDirective, field.astNode) : undefined; | ||||||||||
| const fieldCountBefore = newModel.fields.length; | ||||||||||
|
|
||||||||||
| //If is a basic scalar type | ||||||||||
| const typeClass = getTypeByScalarName(typeString); | ||||||||||
|
|
@@ -211,6 +218,10 @@ export function getAllEntitiesRelations(_schema: GraphQLSchema | string | null): | |||||||||
| } else { | ||||||||||
| throw new Error(`${typeString} is not a valid type`); | ||||||||||
| } | ||||||||||
| if (defaultDirectiveVal) { | ||||||||||
| const packed = newModel.fields.length > fieldCountBefore ? newModel.fields[fieldCountBefore] : undefined; | ||||||||||
| setDefaultValue(entity.name, field.name, packed, defaultDirectiveVal.value, enums.get(typeString)?.values); | ||||||||||
| } | ||||||||||
| // handle indexes | ||||||||||
| if (indexDirectiveVal) { | ||||||||||
| if (typeString !== 'ID' && typeClass) { | ||||||||||
|
|
@@ -354,6 +365,41 @@ function getJoinIndexFields( | |||||||||
| }); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const defaultValuePatterns: Record<string, RegExp> = { | ||||||||||
| [FieldScalar.Int]: /^-?\d+$/, | ||||||||||
| [FieldScalar.BigInt]: /^-?\d+$/, | ||||||||||
| [FieldScalar.Float]: /^-?\d+(\.\d+)?([eE][-+]?\d+)?$/, | ||||||||||
| [FieldScalar.Boolean]: /^(true|false)$/, | ||||||||||
| [FieldScalar.String]: /^/, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| // `@default` exists so a migration can add a non-nullable field to a table that already has rows, so it is only | ||||||||||
| // accepted where that column can be filled with one literal. | ||||||||||
| function setDefaultValue( | ||||||||||
| entityName: string, | ||||||||||
| fieldName: string, | ||||||||||
| field: GraphQLEntityField | undefined, | ||||||||||
| value: string, | ||||||||||
| enumValues: string[] | undefined | ||||||||||
| ): void { | ||||||||||
| const where = `"@default" on ${entityName}.${fieldName}`; | ||||||||||
| if (!field || field.isArray || field.jsonInterface || field.name !== fieldName || field.type === FieldScalar.ID) { | ||||||||||
| throw new Error(`${where} is only supported on scalar and enum fields`); | ||||||||||
| } | ||||||||||
| if (field.nullable) { | ||||||||||
| throw new Error(`${where} is only supported on non-nullable fields`); | ||||||||||
| } | ||||||||||
| const valid = field.isEnum ? enumValues?.includes(value) : defaultValuePatterns[field.type]?.test(value); | ||||||||||
| if (!valid) { | ||||||||||
| throw new Error(`${where}: "${value}" is not a valid ${field.type} value`); | ||||||||||
| } | ||||||||||
| // Int is stored as a Postgres integer. | ||||||||||
| if (field.type === FieldScalar.Int && Math.abs(Number(value)) > 2147483647) { | ||||||||||
| throw new Error(`${where}: "${value}" is not a valid Int value`); | ||||||||||
|
Comment on lines
+397
to
+398
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Accept the full PostgreSQL If a field declares Proposed fix- if (field.type === FieldScalar.Int && Math.abs(Number(value)) > 2147483647) {
+ if (field.type === FieldScalar.Int && (Number(value) < -2147483648 || Number(value) > 2147483647)) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| } | ||||||||||
| field.defaultValue = value; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| function packEntityField( | ||||||||||
| typeString: FieldScalar | string, | ||||||||||
| field: GraphQLField<unknown, unknown>, | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: subquery/subql
Length of output: 6823
🏁 Script executed:
Repository: subquery/subql
Length of output: 17350
🌐 Web query:
site:postgresql.org/docs/current datatype numeric double precision range overflow input 1e999💡 Result:
🏁 Script executed:
rg -n -C 8 'createColumnWithDefaultQuery' packages/node-core packages/utilsRepository: subquery/subql
Length of output: 3436
🏁 Script executed:
rg -n -C 10 'createColumnWithDefaultQuery' packages/node-core/src/db/sync-helper.tsRepository: subquery/subql
Length of output: 993
Reject Float defaults outside PostgreSQL’s
double precisionrange.The Float pattern accepts
1e999, andsetDefaultValuestores it unchanged. Float columns use PostgreSQLdouble precision, and migration passes the value as the new column default. PostgreSQL can reject this value as out of range.Suggested fix
if (!valid) { throw new Error(`${where}: "${value}" is not a valid ${field.type} value`); } + if (field.type === FieldScalar.Float && !Number.isFinite(Number(value))) { + throw new Error(`${where}: "${value}" is not a valid Float value`); + } // Int is stored as a Postgres integer.🤖 Prompt for AI Agents