Skip to content

Commit d8590db

Browse files
committed
fix(vnext): close final completion review gaps
1 parent 8d9a52a commit d8590db

21 files changed

Lines changed: 753 additions & 92 deletions

docs/vnext/marimo-sql-migration.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ reproduce
3838
The same shared service configures one `SqlColumnCatalogProvider`. Each
3939
`loadColumns` call forwards the complete relation request array to one
4040
DataTable metadata batch operation. It does not fetch once per relation.
41-
Relation IDs are the stable IDs emitted by the relation provider. Column IDs
42-
are stable within that relation and connection incarnation.
41+
The column authority resolves each canonical relation path within the same
42+
scope and search paths as relation completion. Column IDs and returned relation
43+
IDs are stable within that connection incarnation.
4344

4445
Each column supplies a canonical identifier and provider-rendered
4546
`insertText`; these are intentionally separate so quoted or dialect-sensitive

src/vnext/__tests__/column-catalog-batch-coordinator.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ function ready(
3838
ordinal: 0,
3939
}],
4040
coverage: "complete",
41-
relationEntityId:
42-
relation.relationEntityId ?? `relation-${relation.requestKey}`,
41+
relationEntityId: `relation-${relation.requestKey}`,
4342
requestKey: relation.requestKey,
4443
status: "ready",
4544
})),

src/vnext/__tests__/column-catalog-boundary.test.ts

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ function request() {
2222
{ path, requestKey: "users" },
2323
{
2424
path: [{ quoted: false, value: "events" }],
25-
relationEntityId: "relation-events",
2625
requestKey: "events",
2726
},
2827
],
@@ -162,7 +161,7 @@ describe("column catalog batch request boundary", () => {
162161
expectedEpoch: epoch,
163162
relations: [
164163
{ path, requestKey: "z" },
165-
{ path, relationEntityId: "stable-a", requestKey: "a" },
164+
{ path, requestKey: "a" },
166165
],
167166
scope: "scope",
168167
searchPaths: [[{ quoted: true, value: "Main" }]],
@@ -171,7 +170,7 @@ describe("column catalog batch request boundary", () => {
171170
status: "accepted",
172171
value: {
173172
relations: [
174-
{ relationEntityId: "stable-a", requestKey: "a" },
173+
{ requestKey: "a" },
175174
{ requestKey: "z" },
176175
],
177176
},
@@ -476,22 +475,6 @@ describe("column catalog response boundary", () => {
476475
],
477476
},
478477
},
479-
{
480-
name: "conflicting known entity",
481-
value: {
482-
epoch,
483-
relations: [
484-
readyResponse().relations[0],
485-
{
486-
columns: [],
487-
coverage: "complete",
488-
relationEntityId: "wrong-events-id",
489-
requestKey: "events",
490-
status: "ready",
491-
},
492-
],
493-
},
494-
},
495478
{
496479
name: "conflicting duplicate column",
497480
value: {

src/vnext/__tests__/column-completion.test.ts

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,18 @@ describe("column completion", () => {
200200
dialect: POSTGRESQL_SQL_RELATION_DIALECT,
201201
outcome: usable([
202202
{
203-
columns: [column("id", "users", 0)],
203+
columns: [
204+
column("id", "users", 0),
205+
Object.freeze({
206+
...column("id", "users", 1),
207+
columnEntityId: "users:id-alternate",
208+
insertText: "id_alternate",
209+
provenance: Object.freeze({
210+
...column("id", "users", 1).provenance,
211+
columnEntityId: "users:id-alternate",
212+
}),
213+
}),
214+
],
204215
coverage: "partial",
205216
relationEntityId: "users",
206217
requestKey: "binding:0",
@@ -390,6 +401,11 @@ describe("column completion", () => {
390401
expect(result).toMatchObject({
391402
sources: [{
392403
coverage: "partial",
404+
failures: [{
405+
code: "unavailable",
406+
requestKey: "binding:1",
407+
retry: "next-request",
408+
}],
393409
outcome: "ready",
394410
}],
395411
value: {
@@ -415,6 +431,81 @@ describe("column completion", () => {
415431
providerId: "columns",
416432
site: current,
417433
})?.sources[0],
418-
).toMatchObject({ outcome: "failed" });
434+
).toMatchObject({
435+
failures: [{
436+
code: "unavailable",
437+
requestKey: "binding:0",
438+
retry: "next-request",
439+
}],
440+
outcome: "failed",
441+
});
442+
});
443+
444+
it("uses deterministic code-unit ordering", () => {
445+
const current = site();
446+
const prepared = prepareSqlColumnCatalogRelations(
447+
current,
448+
POSTGRESQL_SQL_RELATION_DIALECT,
449+
);
450+
const result = composeSqlColumnCompletion({
451+
dialect: POSTGRESQL_SQL_RELATION_DIALECT,
452+
outcome: usable([{
453+
columns: [
454+
column("ä_value", "users", 0),
455+
column("z_value", "users", 1),
456+
],
457+
coverage: "complete",
458+
relationEntityId: "users",
459+
requestKey: "binding:0",
460+
status: "ready",
461+
}]),
462+
prepared,
463+
providerId: "columns",
464+
site: current,
465+
});
466+
467+
expect(result?.value.items.map((item) => item.label)).toEqual([
468+
"z_value",
469+
"ä_value",
470+
]);
471+
});
472+
473+
it("uses relation detail to order columns with equal labels", () => {
474+
const current = site();
475+
const prepared = prepareSqlColumnCatalogRelations(
476+
current,
477+
POSTGRESQL_SQL_RELATION_DIALECT,
478+
);
479+
const result = composeSqlColumnCompletion({
480+
dialect: POSTGRESQL_SQL_RELATION_DIALECT,
481+
outcome: usable([
482+
{
483+
columns: [column("id", "users", 0)],
484+
coverage: "complete",
485+
relationEntityId: "users",
486+
requestKey: "binding:0",
487+
status: "ready",
488+
},
489+
{
490+
columns: [column("id", "orders", 0)],
491+
coverage: "complete",
492+
relationEntityId: "orders",
493+
requestKey: "binding:1",
494+
status: "ready",
495+
},
496+
]),
497+
prepared,
498+
providerId: "columns",
499+
site: current,
500+
});
501+
502+
expect(result?.value.items.map((item) => item.detail)).toEqual([
503+
"VARCHAR — o",
504+
"VARCHAR — u",
505+
]);
506+
expect(result?.value.items.map((item) => item.edit.insert)).toEqual([
507+
"id",
508+
"id",
509+
]);
419510
});
420511
});

src/vnext/__tests__/column-query-site.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,31 @@ describe("recognizeSqlColumnQuerySite", () => {
111111
);
112112
});
113113

114+
it("keeps set-operation arms and JOIN visibility isolated", () => {
115+
const firstArm = ready(
116+
analyze("SELECT | FROM users UNION SELECT x FROM secrets"),
117+
);
118+
expect(firstArm.relations.map((relation) =>
119+
relation.path.at(-1)?.value
120+
)).toEqual(["users"]);
121+
122+
const secondArm = ready(
123+
analyze("SELECT x FROM users UNION SELECT | FROM secrets"),
124+
);
125+
expect(secondArm.relations.map((relation) =>
126+
relation.path.at(-1)?.value
127+
)).toEqual(["secrets"]);
128+
129+
const joinCondition = ready(
130+
analyze(
131+
"SELECT * FROM users u JOIN orders o ON o.user_id = u.| JOIN payments p ON true",
132+
),
133+
);
134+
expect(joinCondition.relations.map((relation) =>
135+
relation.alias?.value
136+
)).toEqual(["u", "o"]);
137+
});
138+
114139
it("supports BigQuery quoted multipart bindings", () => {
115140
const result = ready(
116141
analyze("SELECT t.| FROM `project.dataset.table` AS t", {

src/vnext/__tests__/namespace-catalog-coordinator.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,11 @@ describe("namespace completion composer", () => {
513513
status: "usable",
514514
},
515515
})).toMatchObject({
516-
source: { outcome: "failed" },
516+
source: {
517+
code: "unknown",
518+
outcome: "failed",
519+
retry: "never",
520+
},
517521
value: { issues: ["namespace-catalog-failed"] },
518522
});
519523
expect(composeSqlNamespaceCompletion({

0 commit comments

Comments
 (0)