Skip to content

Commit 05d0f60

Browse files
dmealingclaude
andcommitted
fix(#209): demote a nested INNER view join under a LEFT ancestor (drop-row bug)
Follow-up to 17c26b3. The join type was derived per-hop with no ancestor propagation, but synthesized joins render flat + left-associative — so a required belongs-to hop BELOW a LEFT hop emitted e.g. FROM orders o LEFT OUTER JOIN customers c ON c.id = o.customer_id INNER JOIN countries c0 ON c0.id = c.country_id For an order with no customer, the LEFT yields NULL customer columns, then the INNER's ON (c0.id = c.country_id) references the NULLed c.country_id → no match → the base ORDER ROW IS DROPPED, where pre-#209 it survived with NULLs. Same hazard for a has-many hop followed by a required belongs-to (an aggregate view would drop zero-children base rows it should COALESCE). The single-hop tests didn't cover it. Fix: an INNER hop survives only when its ENTIRE ancestor chain is INNER; otherwise demote to LEFT. Lossless — under a LEFT ancestor, LEFT is the correct type, so nothing is given up. Single-hop joins are unchanged (empty ancestor chain → the `every` guard is vacuously true), so the canonical schema.postgres.sql stays byte-identical. Verified: new nested-chain regression test (nullable→required chain demotes to LEFT; all-required chain stays INNER end to end); codegen-ts 957 pass; typecheck exit 0; canonical schema drift gate byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGQ7oSuNcjhsMHWwZzhBwr
1 parent 17c26b3 commit 05d0f60

2 files changed

Lines changed: 65 additions & 3 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,15 @@ function buildJoinTree(
525525
// FK, or ANY has-many hop (FK on the child — a base row may have zero
526526
// children), stays LEFT OUTER so no base row is dropped.
527527
const fkFieldObj = (fkHolder as MetaObject).findField(fkField);
528+
const selfInner =
529+
referenceHolder === "source" && fkFieldObj !== undefined && isRequired(fkFieldObj);
530+
// Nested-chain safety: joins render flat + left-associative, so an INNER hop
531+
// BELOW any LEFT ancestor drops the base row (its ON references a column the
532+
// LEFT ancestor NULLed). An INNER only survives when the ENTIRE ancestor chain
533+
// is INNER; otherwise demote to LEFT (lossless — under a LEFT ancestor, LEFT is
534+
// the correct type). `path` holds this chain's ancestor hops accumulated so far.
528535
const joinType: "inner" | "left" =
529-
referenceHolder === "source" && fkFieldObj !== undefined && isRequired(fkFieldObj)
530-
? "inner"
531-
: "left";
536+
selfInner && path.every((prior) => prior.joinType === "inner") ? "inner" : "left";
532537

533538
path.push({
534539
entity: currentObj,

server/typescript/packages/codegen-ts/test/projection/join-type-derivation.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,63 @@ describe("#209 — belongs-to join type derived from FK optionality", () => {
6060
});
6161
});
6262

63+
// A required belongs-to hop must NOT be INNER when an ancestor hop in the same
64+
// chain is LEFT: joins render flat + left-associative, so an INNER whose ON
65+
// references a NULLed ancestor column drops the base row (where a LEFT keeps it
66+
// with NULLs). INNER only survives when the ENTIRE ancestor chain is INNER.
67+
describe("#209 — nested join chain: an INNER hop under a LEFT ancestor demotes to LEFT", () => {
68+
// Order --(customerId)--> Customer --(countryId, required)--> Country.
69+
const nestedModel = (orderCustomerIdRequired: boolean) => [
70+
{ "object.entity": { name: "Country", children: [
71+
{ "source.rdb": { "@table": "countries" } },
72+
{ "field.long": { name: "id" } },
73+
{ "field.string": { name: "name" } },
74+
{ "identity.primary": { name: "pk", "@fields": "id" } },
75+
] } },
76+
{ "object.entity": { name: "Customer", children: [
77+
{ "source.rdb": { "@table": "customers" } },
78+
{ "field.long": { name: "id" } },
79+
{ "field.long": { name: "countryId", "@required": true } },
80+
{ "identity.primary": { name: "pk", "@fields": "id" } },
81+
{ "identity.reference": { name: "ref_country", "@fields": "countryId", "@references": "Country" } },
82+
{ "relationship.association": { name: "country", "@objectRef": "Country", "@cardinality": "one" } },
83+
] } },
84+
{ "object.entity": { name: "Order", children: [
85+
{ "source.rdb": { "@table": "orders" } },
86+
{ "field.long": { name: "id" } },
87+
{ "field.long": { name: "customerId", ...(orderCustomerIdRequired ? { "@required": true } : {}) } },
88+
{ "identity.primary": { name: "pk", "@fields": "id" } },
89+
{ "identity.reference": { name: "ref_customer", "@fields": "customerId", "@references": "Customer" } },
90+
{ "relationship.association": { name: "customer", "@objectRef": "Customer", "@cardinality": "one" } },
91+
] } },
92+
{ "object.projection": { name: "OrderView", children: [
93+
{ "source.rdb": { "@kind": "view", "@table": "v_order" } },
94+
{ "field.long": { name: "id", extends: "Order.id" } },
95+
{ "field.string": { name: "countryName", children: [{ "origin.passthrough": { "@from": "Country.name", "@via": "Order.customer.country" } }] } },
96+
{ "identity.primary": { name: "pk", extends: "Order.pk" } },
97+
] } },
98+
];
99+
100+
test("nullable ancestor (Order→Customer LEFT) forces the required Customer→Country hop to LEFT", async () => {
101+
const root = await load(nestedModel(false));
102+
const [v] = buildProjectionViews(root, { dialect: "postgres", columnNamingStrategy: "snake_case" });
103+
// Both joins LEFT OUTER — an order with no customer must survive with NULLs, not
104+
// be dropped by an INNER JOIN countries whose ON references a NULLed customer col.
105+
expect(v!.sql).toContain("LEFT OUTER JOIN customers");
106+
expect(v!.sql).toContain("LEFT OUTER JOIN countries");
107+
expect(v!.sql).not.toContain("INNER JOIN");
108+
});
109+
110+
test("all-required chain stays INNER end to end", async () => {
111+
const root = await load(nestedModel(true));
112+
const [v] = buildProjectionViews(root, { dialect: "postgres", columnNamingStrategy: "snake_case" });
113+
// Order→Customer required + Customer→Country required → both INNER (no base row lost).
114+
expect(v!.sql).toContain("INNER JOIN customers");
115+
expect(v!.sql).toContain("INNER JOIN countries");
116+
expect(v!.sql).not.toContain("LEFT OUTER JOIN");
117+
});
118+
});
119+
63120
describe("#209 — has-many (inverse-FK) join stays LEFT OUTER", () => {
64121
// Program 1→many Week (FK on Week). Even though Week.programId is required, the
65122
// Program→weeks join must stay LEFT OUTER: a program with zero weeks must survive

0 commit comments

Comments
 (0)