mirror of
https://github.com/Kaelio/ktx.git
synced 2026-06-07 07:55:13 +02:00
fix(context): merge overlay columns onto manifest columns by name
composeOverlay was appending overlay columns to the manifest column list, producing duplicate entries when dbt/metabase overlays declared a column just to attach descriptions. The duplicates carried no `type`, so the pydantic SourceDefinition rejected them at semantic-query time and broke `ktx sl query` for every overlay-backed measure. Now overlay columns match base columns by name (case-insensitive): same-name entries merge onto the manifest (overlay fields win, type/role fall back to the base, descriptions merge per source key) and only new names append.
This commit is contained in:
parent
2bca308863
commit
3e12a9fef4
2 changed files with 76 additions and 4 deletions
|
|
@ -139,6 +139,39 @@ describe('composeOverlay', () => {
|
|||
expect(composed.measures).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('merges overlay columns onto same-named manifest columns, preserving manifest type when overlay omits it', () => {
|
||||
const overlay = {
|
||||
name: 'fct_labs',
|
||||
columns: [
|
||||
{ name: 'lab_order_id', descriptions: { user: 'Primary key' } },
|
||||
{ name: 'admin_user_id', descriptions: { user: 'FK to admin_users' } },
|
||||
],
|
||||
};
|
||||
const composed = composeOverlay(baseTable, overlay);
|
||||
// No duplicate columns appended — same-named overlay entries merged onto the base.
|
||||
expect(composed.columns).toHaveLength(3);
|
||||
const labOrder = composed.columns.find((c) => c.name === 'lab_order_id');
|
||||
expect(labOrder?.type).toBe('string');
|
||||
expect(labOrder?.descriptions).toEqual({ user: 'Primary key' });
|
||||
const adminUser = composed.columns.find((c) => c.name === 'admin_user_id');
|
||||
expect(adminUser?.type).toBe('string');
|
||||
expect(adminUser?.descriptions).toEqual({ user: 'FK to admin_users' });
|
||||
});
|
||||
|
||||
it('still appends new overlay computed columns alongside merged same-name columns', () => {
|
||||
const overlay = {
|
||||
name: 'fct_labs',
|
||||
columns: [
|
||||
{ name: 'lab_order_id', descriptions: { user: 'PK doc' } },
|
||||
{ name: 'is_byol', type: 'boolean', expr: "lab_type = 'byol'" },
|
||||
],
|
||||
};
|
||||
const composed = composeOverlay(baseTable, overlay);
|
||||
expect(composed.columns).toHaveLength(4);
|
||||
expect(composed.columns.find((c) => c.name === 'is_byol')?.expr).toBe("lab_type = 'byol'");
|
||||
expect(composed.columns.find((c) => c.name === 'lab_order_id')?.type).toBe('string');
|
||||
});
|
||||
|
||||
it('merges overlay descriptions (plural) with base descriptions keyed by source', () => {
|
||||
const baseWithDescriptions: SemanticLayerSource = {
|
||||
...baseTable,
|
||||
|
|
|
|||
|
|
@ -1367,12 +1367,25 @@ export function composeOverlay(base: SemanticLayerSource, overlay: Record<string
|
|||
|
||||
// Filter out excluded columns
|
||||
const excluded = new Set((normalizedOverlay.exclude_columns as string[] | undefined) ?? []);
|
||||
let columns = result.columns.filter((c) => !excluded.has(c.name));
|
||||
const baseColumns = result.columns.filter((c) => !excluded.has(c.name));
|
||||
|
||||
// Append overlay computed columns
|
||||
// Overlay columns matched by name merge onto the base column (overlay fields win, but
|
||||
// the base column's type/role/etc are preserved when the overlay omits them — dbt-style
|
||||
// overlays often declare a column only to attach descriptions). New names append.
|
||||
const overlayColumns = (normalizedOverlay.columns as SemanticLayerSource['columns'] | undefined) ?? [];
|
||||
columns = [...columns, ...overlayColumns];
|
||||
result.columns = columns;
|
||||
const baseByName = new Map(baseColumns.map((c) => [c.name.toLowerCase(), c]));
|
||||
const mergedAppended: SemanticLayerSource['columns'] = [];
|
||||
const mergedByName = new Map<string, SemanticLayerSource['columns'][number]>();
|
||||
for (const overlay of overlayColumns) {
|
||||
const key = overlay.name.toLowerCase();
|
||||
const base = baseByName.get(key);
|
||||
if (base) {
|
||||
mergedByName.set(key, mergeOverlayColumn(base, overlay));
|
||||
} else {
|
||||
mergedAppended.push(overlay);
|
||||
}
|
||||
}
|
||||
result.columns = [...baseColumns.map((c) => mergedByName.get(c.name.toLowerCase()) ?? c), ...mergedAppended];
|
||||
|
||||
// Measures from overlay only
|
||||
result.measures = (normalizedOverlay.measures as SemanticLayerSource['measures'] | undefined) ?? [];
|
||||
|
|
@ -1432,6 +1445,32 @@ function parseJoinOn(
|
|||
return { fromColumn: leftCol, toColumn: rightCol };
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge an overlay column declaration onto a matching manifest column. Overlay fields
|
||||
* win, except descriptions (plural) which merge per source key. Manifest values are
|
||||
* preserved when the overlay omits them — this lets dbt/metabase emit description-only
|
||||
* overlay column entries without redeclaring `type:` (which would have to mirror the
|
||||
* scan column and rot when the schema changes).
|
||||
*/
|
||||
function mergeOverlayColumn(
|
||||
base: SemanticLayerSource['columns'][number],
|
||||
overlay: SemanticLayerSource['columns'][number],
|
||||
): SemanticLayerSource['columns'][number] {
|
||||
const merged: SemanticLayerSource['columns'][number] = { ...base, ...overlay };
|
||||
if (!overlay.type && base.type) {
|
||||
merged.type = base.type;
|
||||
}
|
||||
if (!overlay.role && base.role) {
|
||||
merged.role = base.role;
|
||||
}
|
||||
const baseDescriptions = base.descriptions ?? {};
|
||||
const overlayDescriptions = overlay.descriptions ?? {};
|
||||
if (Object.keys(baseDescriptions).length > 0 || Object.keys(overlayDescriptions).length > 0) {
|
||||
merged.descriptions = { ...baseDescriptions, ...overlayDescriptions };
|
||||
}
|
||||
return merged;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fill any blank `type`, `descriptions`, or `role` on the source's columns from the
|
||||
* matching manifest column (by name). Local values always win. Columns absent from
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue