Skip to content

Commit b854bd5

Browse files
dmealingclaude
andcommitted
fix(codegen): a renamed projection field built a view over a column that does not exist
A projection field may rename what it exposes: { field.string: { name: bookingRef, extends: "Shipment.reference" } } The view's output alias comes from the projection field; the column it SELECTs must come from the `extends` target. Both were derived from the projection field, so the emitted view read `SELECT s.booking_ref` from a table whose column is `reference`, and the GROUP BY repeated it. SQLite does not resolve a view body at CREATE VIEW, so `meta migrate --apply` reported success and the view failed at the first SELECT; Postgres rejected the migration outright. The source column now resolves through the `extends` target, and only when that target names the BASE entity. Two shapes must NOT be read as renames: a bare `extends` onto a package-level abstract is shape reuse (`refNamedOwner` returns undefined), and a read-view HOST reads its own table, where a dotted `extends` onto a sibling field is shape reuse too — treating that as a rename would serve the sibling's data under this field's name, which is valid SQL and wrong rows. `buildGroupBy` maps `sourceColumn`, so the GROUP BY is corrected by the same change, and migrate-ts consumes codegen-ts's `buildProjectionViews` rather than generating view DDL of its own, so there was one door to fix. Existing tests only ever bound same-named fields, which is why this was never caught. Renaming a base column is one of the two things the authoring guidance names as genuinely forcing a projection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JGmCLHbQKuLBPjjhWC43jk
1 parent 70e29fa commit b854bd5

3 files changed

Lines changed: 296 additions & 6 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ edit (two registered `description` strings) and was ruled a hold, as 1.0.4's was
4141
This covered BOTH view hosts — an `object.projection` and a write-through entity's replica
4242
view render through the same function, and the read-view is the shape the authoring
4343
guidance recommends over a projection.
44+
- **A projection field that RENAMED the base field built a view over a column that does not
45+
exist.** `{ field.string: { name: bookingRef, extends: "Shipment.reference" } }` emitted
46+
`SELECT s.booking_ref` against a table whose column is `reference`: the view's output alias
47+
and the column it selects were both derived from the projection's field, and the `extends`
48+
target was never consulted. SQLite does not resolve a view body at `CREATE VIEW`, so
49+
`meta migrate --apply` reported success and the view failed at the first `SELECT`;
50+
Postgres rejected the migration outright. The source column now comes from the `extends`
51+
target, and only when that target names the BASE entity — a bare `extends` onto a
52+
package-level abstract is shape reuse, not a column binding, and a read-view HOST reads
53+
its own table, where a dotted `extends` onto a sibling field is shape reuse too. Both
54+
still resolve to the field's own name. Renaming a base column is one of the two things the
55+
authoring guidance names as genuinely forcing a projection. **An adopter whose view had a
56+
renamed passthrough will see a view-replace in the next `meta migrate` diff** — that is the
57+
corrected SQL, and the view it replaces could not be queried.
4458
- **`meta eject` and `meta gen --list` print an install command you can paste.** The line
4559
was documented as paste-ready and was not, on the most common selection
4660
(`meta eject entity queries routes names barrel`):

‎server/typescript/packages/codegen-ts/src/projection/extract-view-spec.ts‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,50 @@ function sourceColumnNameFor(
570570
return columnNameFromField(entityField.name, ctx.columnNamingStrategy);
571571
}
572572

573+
/**
574+
* The BASE table's column an extends-bound passthrough field SELECTS.
575+
*
576+
* A projection field may RENAME what it exposes
577+
* (`{ field.string: { name: bookingRef, extends: "Shipment.reference" } }`). The view's
578+
* OUTPUT alias is the projection field's own; the column it reads from the base table is
579+
* the extends TARGET's. Deriving both from the projection field emits
580+
* `SELECT s.booking_ref` against a table whose column is `reference` — SQL that SQLite
581+
* accepts at CREATE VIEW and only fails at the first SELECT, and that Postgres rejects
582+
* outright at migrate time.
583+
*
584+
* Caller contract: this is for a PLAIN projection only. A read-view HOST (`base ===
585+
* projection`) reads its own table, so its passthrough source is always the field's own
586+
* column and there is no rename to resolve — and a host field may legally `extends` a
587+
* SIBLING field of the same entity for shape reuse, which this would otherwise read as a
588+
* rename and silently serve the sibling's data under this field's name.
589+
*
590+
* Only a DOTTED extends naming the BASE entity redirects the source column:
591+
*
592+
* - `extends: "Shipment.reference"` — binds a base column; the target's column wins.
593+
* - `extends: "Code24"` (bare, a package-level abstract) — SHAPE REUSE, not a binding.
594+
* `refNamedOwner` returns undefined, so the field's own name stays the source.
595+
* - `extends: "Other.foo"` — names an entity that is not the base. The caller emits this
596+
* column against `joinTree.baseAlias`, so borrowing another entity's column name would
597+
* be a second wrong answer; keep the field's own.
598+
*
599+
* The alias is not always the projection's spelling: `sourceColumnNameFor` reads `@column`
600+
* with the RESOLVING accessor, so a renaming field whose base carries an explicit `@column`
601+
* inherits it and both sides land on that physical name. The rename then holds on the TS /
602+
* wire tier only. That is pre-existing and self-consistent — the Drizzle view decl binds
603+
* the same column — but it is not a promise this function makes.
604+
*/
605+
function baseColumnNameFor(
606+
field: MetaField,
607+
base: MetaObject,
608+
root: MetaRoot,
609+
ctx: ExtractContext,
610+
): string {
611+
const target = field.superData;
612+
if (target === undefined) return sourceColumnNameFor(field, ctx);
613+
if (refNamedOwner(field, root) !== base) return sourceColumnNameFor(field, ctx);
614+
return sourceColumnNameFor(target, ctx);
615+
}
616+
573617
/**
574618
* Physical column for a join FK/PK field — resolves @column + naming strategy the
575619
* same way passthrough columns do (EFFECTIVE fields, so inherited PKs resolve).
@@ -976,22 +1020,27 @@ function buildSelectSpec(
9761020
// fields — the declared set IS the exposure (FR-024/ADR-0028). Either way, each
9771021
// field's own origin decides passthrough-from-base vs derived-from-join. origin.*
9781022
// NEVER inherits (ADR-0029), so the origin reads below are own (category 4).
979-
const declaredFields: MetaField[] =
980-
base === projection
981-
? base.fields()
982-
: projection.ownChildren().filter((c): c is MetaField => c.type === TYPE_FIELD);
1023+
// A read-view HOST reads its OWN table (base === projection), so every passthrough
1024+
// sources from the field's own column; a plain projection reads the base's, which an
1025+
// extends-bound field may rename. The flag decides both the field set and the source.
1026+
const isReadViewHost = base === projection;
1027+
const declaredFields: MetaField[] = isReadViewHost
1028+
? base.fields()
1029+
: projection.ownChildren().filter((c): c is MetaField => c.type === TYPE_FIELD);
9831030
for (const field of declaredFields) {
9841031
const origin = field.ownChildren().find((c) => c.type === TYPE_ORIGIN);
9851032
const dbCol = sourceColumnNameFor(field, ctx);
9861033

9871034
if (!origin) {
988-
// Declared on projection but no origin — passthrough from base table.
1035+
// Declared on projection but no origin — passthrough from base table. The alias is
1036+
// this field's own column; the SOURCE is the extends target's, which differs
1037+
// whenever the projection renames the base field.
9891038
columns.push({
9901039
kind: "passthrough",
9911040
fieldName: field.name,
9921041
dbColAlias: dbCol,
9931042
sourceAlias: joinTree.baseAlias,
994-
sourceColumn: dbCol,
1043+
sourceColumn: isReadViewHost ? dbCol : baseColumnNameFor(field, base, root, ctx),
9951044
});
9961045
continue;
9971046
}
Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
import { describe, test, expect } from "bun:test";
2+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
3+
import { extractViewSpec } from "../../src/projection/extract-view-spec.js";
4+
import { emitViewDdl } from "../../src/projection/view-ddl-emit.js";
5+
6+
/**
7+
* A projection field may RENAME what it exposes:
8+
*
9+
* { field.string: { name: bookingRef, extends: "Shipment.reference" } }
10+
*
11+
* The view's OUTPUT column is the projection field's (`booking_ref`); the column it
12+
* SELECTS is the extends TARGET's (`reference`). Deriving both from the projection
13+
* field emits `SELECT s.booking_ref` against a table whose column is `reference` —
14+
* a view SQLite accepts at CREATE and fails at first SELECT, Postgres rejects outright.
15+
*
16+
* Renaming base columns is one of only two things the authoring guidance says
17+
* genuinely forces a projection, so this is the construct's headline case.
18+
*/
19+
async function load(children: unknown[]) {
20+
const json = JSON.stringify({ "metadata.root": { package: "test", children } });
21+
const result = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
22+
if (result.errors.length > 0) {
23+
throw new Error(`Loader errors:\n${result.errors.map((e) => e.message).join("\n")}`);
24+
}
25+
return result.root;
26+
}
27+
28+
const shipment = {
29+
"object.entity": {
30+
name: "Shipment",
31+
children: [
32+
{ "source.rdb": { "@table": "shipment" } },
33+
{ "field.int": { name: "id" } },
34+
{ "field.string": { name: "reference" } },
35+
{ "field.string": { name: "originCity" } },
36+
{ "identity.primary": { name: "pk", "@fields": "id" } },
37+
],
38+
},
39+
};
40+
41+
describe("extractViewSpec — extends-bound passthrough that RENAMES the base field", () => {
42+
test("sourceColumn comes from the extends target, dbColAlias from the projection field", async () => {
43+
const root = await load([
44+
shipment,
45+
{
46+
"object.projection": {
47+
name: "ShipmentSummary",
48+
children: [
49+
{ "source.rdb": { "@kind": "view", "@view": "v_shipment_summary" } },
50+
{ "field.int": { name: "id", extends: "Shipment.id" } },
51+
// THE RENAME: exposed as bookingRef, sourced from Shipment.reference.
52+
{ "field.string": { name: "bookingRef", extends: "Shipment.reference" } },
53+
{ "identity.primary": { name: "pk", extends: "Shipment.pk" } },
54+
],
55+
},
56+
},
57+
]);
58+
59+
const projection = root.objects().find((o) => o.name === "ShipmentSummary")!;
60+
const spec = extractViewSpec(projection, root, { columnNamingStrategy: "snake_case" });
61+
62+
const booking = spec.selectSpec.columns.find((c) => c.fieldName === "bookingRef")!;
63+
expect(booking.kind).toBe("passthrough");
64+
if (booking.kind === "passthrough") {
65+
// The view exposes it under the projection's own name…
66+
expect(booking.dbColAlias).toBe("booking_ref");
67+
// …but SELECTs the base entity's real column.
68+
expect(booking.sourceColumn).toBe("reference");
69+
}
70+
71+
// An un-renamed field is unaffected.
72+
const id = spec.selectSpec.columns.find((c) => c.fieldName === "id")!;
73+
if (id.kind === "passthrough") {
74+
expect(id.dbColAlias).toBe("id");
75+
expect(id.sourceColumn).toBe("id");
76+
}
77+
});
78+
79+
test("an explicit @column on the projection field names the VIEW column, not the base's", async () => {
80+
const root = await load([
81+
shipment,
82+
{
83+
"object.projection": {
84+
name: "ShipmentSummary",
85+
children: [
86+
{ "source.rdb": { "@kind": "view", "@view": "v_shipment_summary" } },
87+
{ "field.int": { name: "id", extends: "Shipment.id" } },
88+
{
89+
"field.string": {
90+
name: "bookingRef",
91+
extends: "Shipment.reference",
92+
"@column": "booking_code",
93+
},
94+
},
95+
{ "identity.primary": { name: "pk", extends: "Shipment.pk" } },
96+
],
97+
},
98+
},
99+
]);
100+
101+
const projection = root.objects().find((o) => o.name === "ShipmentSummary")!;
102+
const spec = extractViewSpec(projection, root, { columnNamingStrategy: "snake_case" });
103+
const booking = spec.selectSpec.columns.find((c) => c.fieldName === "bookingRef")!;
104+
if (booking.kind === "passthrough") {
105+
expect(booking.dbColAlias).toBe("booking_code");
106+
expect(booking.sourceColumn).toBe("reference");
107+
}
108+
});
109+
110+
test("a field-level extends onto an ABSTRACT (shape reuse) does not hijack the source column", async () => {
111+
const root = await load([
112+
// A package-level abstract field reused for its shape, NOT a base column.
113+
{ "field.string": { name: "Code24", abstract: true, "@maxLength": 24 } },
114+
{
115+
"object.entity": {
116+
name: "Shipment",
117+
children: [
118+
{ "source.rdb": { "@table": "shipment" } },
119+
{ "field.int": { name: "id" } },
120+
{ "field.string": { name: "reference" } },
121+
{ "identity.primary": { name: "pk", "@fields": "id" } },
122+
],
123+
},
124+
},
125+
{
126+
"object.projection": {
127+
name: "ShipmentSummary",
128+
children: [
129+
{ "source.rdb": { "@kind": "view", "@view": "v_shipment_summary" } },
130+
{ "field.int": { name: "id", extends: "Shipment.id" } },
131+
// Extends an abstract for SHAPE — the base column is its own name.
132+
{ "field.string": { name: "reference", extends: "Code24" } },
133+
{ "identity.primary": { name: "pk", extends: "Shipment.pk" } },
134+
],
135+
},
136+
},
137+
]);
138+
139+
const projection = root.objects().find((o) => o.name === "ShipmentSummary")!;
140+
const spec = extractViewSpec(projection, root, { columnNamingStrategy: "snake_case" });
141+
const ref = spec.selectSpec.columns.find((c) => c.fieldName === "reference")!;
142+
if (ref.kind === "passthrough") {
143+
expect(ref.dbColAlias).toBe("reference");
144+
// NOT "code24" — the abstract is shape, not a base column.
145+
expect(ref.sourceColumn).toBe("reference");
146+
}
147+
});
148+
149+
test("end to end: the emitted CREATE VIEW selects the base column under the new alias", async () => {
150+
const root = await load([
151+
shipment,
152+
{
153+
"object.projection": {
154+
name: "ShipmentSummary",
155+
children: [
156+
{ "source.rdb": { "@kind": "view", "@view": "v_shipment_summary" } },
157+
{ "field.int": { name: "id", extends: "Shipment.id" } },
158+
{ "field.string": { name: "bookingRef", extends: "Shipment.reference" } },
159+
{ "field.string": { name: "originCity", extends: "Shipment.originCity" } },
160+
{ "identity.primary": { name: "pk", extends: "Shipment.pk" } },
161+
],
162+
},
163+
},
164+
]);
165+
166+
const projection = root.objects().find((o) => o.name === "ShipmentSummary")!;
167+
const spec = extractViewSpec(projection, root, { columnNamingStrategy: "snake_case" });
168+
const sql = emitViewDdl(spec, {
169+
dialect: "sqlite",
170+
baseTableName: "shipment",
171+
joinTables: {},
172+
});
173+
174+
// The rename is an ALIAS over the base column, not a second column name.
175+
expect(sql).toContain("s.reference AS booking_ref");
176+
// And never the pre-fix form, which named a column `shipment` does not have.
177+
expect(sql).not.toContain("s.booking_ref");
178+
expect(sql).toContain("s.origin_city AS origin_city");
179+
});
180+
});
181+
182+
describe("read-view HOST — the rename rule must NOT apply", () => {
183+
/**
184+
* A write-through entity's read-view reads its OWN table, so a passthrough always sources
185+
* from the field's own column. A host field may legally `extends` a SIBLING field of the
186+
* same entity for shape reuse — a dotted ref whose owner IS the base. Treating that as a
187+
* rename makes the view serve the sibling's data under this field's name: valid SQL, no
188+
* error, wrong rows.
189+
*/
190+
test("a host field extending a SIBLING field still selects its own column", async () => {
191+
const root = await load([
192+
{
193+
"object.entity": {
194+
name: "Shipment",
195+
children: [
196+
{ "source.rdb": { "@kind": "table", "@table": "shipment", "@role": "primary" } },
197+
{ "source.rdb": { "@kind": "view", "@view": "v_shipment", "@role": "replica" } },
198+
{ "field.int": { name: "id" } },
199+
{ "field.string": { name: "reference" } },
200+
// Shape reuse off a sibling — NOT a rename of it.
201+
{ "field.string": { name: "altReference", extends: "Shipment.reference" } },
202+
{ "identity.primary": { name: "pk", "@fields": "id" } },
203+
],
204+
},
205+
},
206+
]);
207+
208+
const host = root.objects().find((o) => o.name === "Shipment")!;
209+
const spec = extractViewSpec(host, root, { columnNamingStrategy: "snake_case" });
210+
211+
const alt = spec.selectSpec.columns.find((c) => c.fieldName === "altReference")!;
212+
expect(alt.kind).toBe("passthrough");
213+
if (alt.kind === "passthrough") {
214+
expect(alt.dbColAlias).toBe("alt_reference");
215+
// Its OWN column — not the sibling's, which is a real separate column on the table.
216+
expect(alt.sourceColumn).toBe("alt_reference");
217+
}
218+
219+
const sql = emitViewDdl(spec, {
220+
dialect: "sqlite",
221+
baseTableName: "shipment",
222+
joinTables: {},
223+
});
224+
expect(sql).toContain("s.alt_reference AS alt_reference");
225+
expect(sql).toContain("s.reference AS reference");
226+
});
227+
});

0 commit comments

Comments
 (0)