🐛 fix(grid): show column comments for joined (multi-source) query results - #6358
Conversation
…ults Joined results were marked non-editable (no single tableMeta), so the data grid's comment map built only from tableMeta.columns stayed empty and every column comment disappeared. The source tables' metadata was already loaded during editability analysis — it was just discarded. - QueryTab gains resultColumnComments (merged comments from every JOIN source) and queryDisplaySourceColumns (display-only result->source mapping), populated in buildQueryMetadataPatch for multi-source branches. - DataGrid merges resultColumnComments into columnCommentMap and resolves comments through queryDisplaySourceColumns first. - ContentArea passes the new props; tab clone and result-cache snapshot carry the fields. - Tests: multi-source JOIN comment merge + display mapping (queryStore), per-source column mapping (sqlAnalysis), DataGrid source assertions. Validation: vue-tsc typecheck, oxlint, oxfmt check, 45 related test files / 983 tests pass; 4 pre-existing failures (Redis completion, grid popover/condition/readonly specs) confirmed failing before these changes.
f58b228 to
b51ceb7
Compare
t8y2
left a comment
There was a problem hiding this comment.
Request changes: the result-column comment mapping loses source identity and therefore returns incorrect metadata.
- Comments from all source tables are merged into one map keyed only by physical column name with first-source-wins behavior. Two tables with
idcolumns can display the first table's comment for both result columns. - The result mapping stores a source column name but not its source key, and it uses unbound SQL analysis. A uniquely resolvable unqualified alias such as
name AS usernamecannot map back to the physical column even though the existing metadata binder can resolve it. - The new fields are written to the result cache snapshot but are not restored when the cached result is reloaded.
- Lowercase aliases in the global map also collapse valid quoted mixed-case identifiers.
Please store comments per result ordinal with source identity, reuse database-aware binding/canonicalization, return no comment for ambiguity, restore the fields from cache, and cover same-name sources, aliases, quoted case, duplicate result names, and cache restore.
Reworks the multi-source column-comment fix per review on t8y2#6358. - resultColumnComments is now indexed by result ordinal (Array<string|undefined>) instead of a first-source-wins name map: two tables with an id column keep per-source comments on both result columns. - queryDisplaySourceColumns now carries source identity per ordinal ({sourceKey, sourceColumn}) via the same database-aware binder (resolveMetadataColumnName) used by editability analysis: uniquely resolvable unqualified aliases (name AS username) map back to the physical column, quoted mixed-case identifiers match exactly, and ambiguous or computed columns yield undefined instead of a wrong comment. - The two fields round-trip through the result cache snapshot and are restored on reload (restoreCachedResultPayload, projectResultRun, persistResultRun, captureDisplayedResultRun, clearResultRunPayload) and reset on every execution state transition. - DataGrid consumes comments per result ordinal and keeps the tableMeta-derived lookup fallback for single-source grids. Validation: vue-tsc, oxlint; queryStore.multiSourceColumnComments (6), multiSourceColumnMapping (9), tabResultCache (incl. cache restore), DataGridColumnComments pass; sql+tabs suites 991 tests, queryStore suites 197/198 (1 pre-existing tableMetaRestore timeout confirmed on HEAD baseline); 3 pre-existing grid spec load failures confirmed on HEAD baseline.
|
Addressed the review feedback in
I also added regression coverage for duplicate source column names, aliases / ambiguity, quoted mixed-case identifiers, duplicate result names, and cache round trips. |
t8y2
left a comment
There was a problem hiding this comment.
维护者补丁 622eef73e 已推送。
根因:复制查询 Tab 时对数组 resultColumnComments 使用对象展开,运行时会得到普通对象,破坏按列序号读取注释的数组语义。
修复:改为数组展开,并增加复制 Tab 时保持数组类型、内容及独立引用的回归测试。
验证:字段注释、来源映射、结果缓存和 Tab 复制相关 5 个测试文件共 42 项通过;pnpm typecheck、格式检查及 git diff --check 通过。未新增依赖。
t8y2
left a comment
There was a problem hiding this comment.
Reviewed the maintainer-patched exact head 622eef73e; column-comment regressions, typecheck, and frontend CI pass.
|
Thanks for the contribution! Merged in 64e5e88, will be released in the next version. |
Fixes #2129 (first half: result-set column comments for joined queries).
Problem
Executing a JOIN / multi-source query hid every column comment in the result grid. Single-table queries worked because the data grid builds its comment map from
tableMeta.columns; multi-source results are not editable sotableMetawasundefined, and the source tables' metadata — already loaded during editability analysis — was discarded.Changes
QueryTab(types/database.ts) gainsresultColumnComments(merged comments from every JOIN source) andqueryDisplaySourceColumns(display-only result→source mapping).queryStore.tsbuildQueryMetadataPatchnow populates both fields for multi-source branches viamergeEditableSourceColumnComments/mergeEditableSourceDisplayColumns;applyQueryMetadataPatch, tab clone and result-cache snapshot carry them.DataGrid.vuemergesresultColumnCommentsintocolumnCommentMapand resolves header/tooltip comments throughqueryDisplaySourceColumnsfirst for precise matching.ContentArea.vuepasses the new props.Tests
queryStore.multiSourceColumnComments.spec.ts: JOIN result merges comments from both sources + display mapping; single-source regression stays free of the new fields.multiSourceColumnMapping.spec.ts: sqlAnalysis per-source column mapping (incl. duplicate result names).DataGridColumnComments.spec.ts: extended source assertions.Validation
vue-tsctypecheck ✅,oxlint✅,oxfmt --check✅Follow-up
The second half of #2129 — hovering a column/table name in the SQL editor to show its comment — will be a separate PR.
Review follow-up
This update addresses the requested changes from the latest review:
sourceKey+sourceColumn, so same-named columns from joined sources do not collide.Regression coverage includes: