diff --git a/.changeset/preview-omits-virtual-fields.md b/.changeset/preview-omits-virtual-fields.md new file mode 100644 index 0000000000..e180b48617 --- /dev/null +++ b/.changeset/preview-omits-virtual-fields.md @@ -0,0 +1,21 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): `os migrate plan` no longer promises columns the apply can never create (#3978) + +`previewDeferredSchemaWork()` listed every declared field name when computing +pending `create_table` / `add_columns` work, but `createColumn` returns early +for a virtual `formula` field — no column is ever created for it. + +So a formula field showed up as pending `add_columns` that `apply` reported as +performed without doing anything, and the very next `plan` reported it again. +A freshly-applied database looked permanently un-migrated, with no invocation +able to clear the finding. On `examples/app-crm` that was 4 columns +(`crm_contact.full_name`, `crm_lead.is_closed`, `crm_opportunity.expected_revenue`, +`crm_opportunity.days_to_close`) reported forever. + +The preview now filters through `fieldHasColumn` — the same helper `createColumn` +and the column differ already answer "does this field materialize a column?" +with — so the plan and the flush cannot disagree. `multiple` fields are +unaffected: they materialize as a JSON column and are still reported. diff --git a/packages/plugins/driver-sql/src/schema-drift.ts b/packages/plugins/driver-sql/src/schema-drift.ts index f328def2b9..5b3adecd65 100644 --- a/packages/plugins/driver-sql/src/schema-drift.ts +++ b/packages/plugins/driver-sql/src/schema-drift.ts @@ -110,6 +110,11 @@ export interface PendingSchemaWork { /** * Declared columns for a create; the missing ones for an add; the columns * being converged for the two datetime steps. + * + * The additive kinds name only fields that MATERIALIZE a column + * ({@link fieldHasColumn}) — a virtual `formula` field never appears. The + * promise above cuts both ways: a plan may not promise work `apply` cannot + * deliver either, or the finding can never be cleared (#3978). */ columns: string[]; /** diff --git a/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts b/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts index dfc8141292..f6afbb67aa 100644 --- a/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-deferred-ddl.test.ts @@ -136,4 +136,79 @@ describe('SqlDriver deferred schema DDL (#3917)', () => { expect(driver.deferredSchemaObjectCount).toBe(0); expect(await driver.previewDeferredSchemaWork()).toEqual([]); }); + + // ── #3978: the plan may only promise work the flush can deliver ─────────── + // + // The mirror image of #3954. That one closed "the plan understates what apply + // does"; this closes "the plan promises what apply cannot do". `createColumn` + // returns early for a virtual `formula` field — no column is ever created — + // but the preview listed every declared field name regardless, so a formula + // field showed up as pending `add_columns` that `apply` reported as performed + // without doing anything, and the next `plan` reported it again, forever. + // Both sides must answer "does this field become a column?" the same way. + describe('virtual fields are never promised as pending work (#3978)', () => { + const CONTACT = { + name: 'contacts', + fields: { + first_name: { type: 'text' }, + last_name: { type: 'text' }, + // Virtual — computed on read, no physical column (see createColumn). + full_name: { type: 'formula', expression: 'record.first_name' }, + // `multiple` is NOT virtual: it materializes as a JSON column. + tags: { type: 'text', multiple: true }, + }, + }; + + it('omits the formula field from a create_table preview, keeps the JSON column', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([CONTACT]); + + expect(await driver.previewDeferredSchemaWork()).toEqual([ + { table: 'contacts', kind: 'create_table', columns: ['first_name', 'last_name', 'tags'] }, + ]); + }); + + it('omits it from an add_columns preview too', async () => { + driver = makeDriver(); + await driver.initObjects([{ name: 'contacts', fields: { first_name: { type: 'text' } } }]); + + driver.setDeferredDdl(true); + await driver.initObjects([CONTACT]); + + expect(await driver.previewDeferredSchemaWork()).toEqual([ + { table: 'contacts', kind: 'add_columns', columns: ['last_name', 'tags'] }, + ]); + }); + + it('converges: after a flush the next preview is empty', async () => { + // The defect proper. Pre-fix the second preview still reported + // `add_columns: ['full_name']` — a freshly-applied database looking + // permanently un-migrated, with no apply able to clear it. + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([CONTACT]); + await driver.flushDeferredSchemaDdl(); + + driver.setDeferredDdl(true); + await driver.initObjects([CONTACT]); + expect(await driver.previewDeferredSchemaWork()).toEqual([]); + }); + + it('what flush REPORTS having done matches the columns it actually created', async () => { + driver = makeDriver(); + driver.setDeferredDdl(true); + await driver.initObjects([CONTACT]); + + const performed = await driver.flushDeferredSchemaDdl(); + const create = performed.find((p) => p.kind === 'create_table')!; + const physical = new Set(Object.keys(await (driver as any).knex('contacts').columnInfo())); + + // Every promised column exists... + for (const c of create.columns) expect(physical.has(c)).toBe(true); + // ...and the virtual one was neither promised nor created. + expect(create.columns).not.toContain('full_name'); + expect(physical.has('full_name')).toBe(false); + }); + }); }); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 2b4e31c4ab..66767fb567 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -20,6 +20,7 @@ import { diffManagedTable, driftKey, expectedIndexes, + fieldHasColumn, isIndexDriftOp, legacyUniqueReplacements, uniqueIndexesFromFields, @@ -2864,11 +2865,22 @@ export class SqlDriver implements IDataDriver { * a database that has not been migrated yet. That is the right trade for a * command the operator ran to be told the size of the job — and it is paid * once, by `plan`/`apply`, never on a normal boot. + * + * The obligation runs both ways (#3978). #3954 closed "the plan understates + * what apply does"; this closes its mirror image — the plan must not promise + * work apply CANNOT do. Only fields that materialize a column are listed, + * decided by {@link fieldHasColumn}, the same helper `createColumn` and the + * column differ use. A virtual `formula` field has no column, so listing it + * produced an `add_columns` entry `apply` reported as performed without doing + * anything and the next `plan` reported again: a finding no invocation could + * ever clear, making a freshly-applied database look un-migrated. */ async previewDeferredSchemaWork(): Promise { const out: PendingSchemaWork[] = []; for (const [tableName, obj] of this.deferredSchemaObjects) { - const declared = Object.keys(obj.fields ?? {}); + const declared = Object.entries(obj.fields ?? {}) + .filter(([, field]) => fieldHasColumn(field ?? {})) + .map(([name]) => name); if (!(await this.knex.schema.hasTable(tableName))) { // A table that does not exist yet is created empty, so nothing to converge. out.push({ table: tableName, kind: 'create_table', columns: declared });