fix(driver-sql): the plan must not promise columns the apply can never create (#3978)

.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.

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[];
  /**

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);
+ });
+ });
 });

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<PendingSchemaWork[]> {
  const out: PendingSchemaWork[] = [];
  for (const [tableName, obj] of this.deferredSchemaObjects) {
- const declared = Object.keys(obj.fields ?? {});
+ const declared = Object.entries<any>(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 });