Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fromquery-select-all.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@maple-dev/effect-orm": patch
---

A bare `select()` over a subquery or union (`CH.fromQuery(inner, "w").select()`, `CH.fromUnion(union, "u").select()`) selects every alias the source selects, as its type already said. It died with `QueryBuilderDefect: select() is required`, because the derived source has no column list until its own SELECT is evaluated.
17 changes: 14 additions & 3 deletions src/ch/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ function compileInner<
const $ = makeAccessor(state)

// SELECT
const selectExprs = state.selectFn ? state.selectFn($) : {}
const selectExprs = selectRecordOf(state, $) ?? {}
const keys = Object.keys(selectExprs)
if (
options?.selectKeys &&
Expand Down Expand Up @@ -1271,8 +1271,19 @@ export function selectedAliasesOf(query: CHQuery<any, any, any>): ReadonlyArray<

/** Evaluate a query's SELECT callback without compiling it. */
function selectExprsOf(query: CHQuery<any, any, any>): Record<string, unknown> | undefined {
const state = query._state
return state.selectFn ? state.selectFn(makeAccessor(state)) : undefined
return selectRecordOf(query._state, makeAccessor(query._state))
}

/**
* A query's SELECT list against accessor `$`: its callback's record, or for a
* bare `select()` over a subquery or union, every alias that source selects.
*/
function selectRecordOf(state: CHQueryState, $: any): Record<string, any> | undefined {
if (state.selectAll) {
const source = state.fromUnion ? unionExprsOf(state.fromUnion._state.queries) : state.fromQuery ? selectExprsOf(state.fromQuery) : undefined
return source === undefined ? undefined : Object.fromEntries(Object.keys(source).map((alias) => [alias, $[alias]]))
}
return state.selectFn?.($)
}

/**
Expand Down
16 changes: 13 additions & 3 deletions src/ch/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ export interface CHQueryState {
readonly tableAlias?: string
readonly columns: ColumnDefs
readonly selectFn?: ($: any) => SelectRecord
/**
* A bare `select()` over a subquery or union: every alias the source
* selects. Its columns are only known once the source's SELECT is evaluated,
* which `compile` does, so the expansion waits for it.
*/
readonly selectAll?: true
readonly whereFn?: ($: any) => ReadonlyArray<Condition | undefined>
readonly groupByKeys: string[]
/** Post-aggregation filter. Deliberately NOT consulted when deriving tenant
Expand Down Expand Up @@ -569,11 +575,15 @@ function makeQuery<
_state: state,

select(...args: any[]): any {
if (args.length === 0) return makeQuery({ ...state, selectFn: selectEvery(Object.keys(state.columns)) })
if (args.length === 0) {
return state.fromQuery !== undefined || state.fromUnion !== undefined
? makeQuery({ ...state, selectFn: undefined, selectAll: true })
: makeQuery({ ...state, selectFn: selectEvery(Object.keys(state.columns)) })
}
// String overload: select("Col1", "Col2") → select($ => ({ Col1: $.Col1, Col2: $.Col2 }))
if (typeof args[0] === "string") return makeQuery({ ...state, selectFn: selectEvery(args as string[]) })
if (typeof args[0] === "string") return makeQuery({ ...state, selectFn: selectEvery(args as string[]), selectAll: undefined })
// Callback overload: select($ => ({ ... }))
return makeQuery({ ...state, selectFn: args[0] })
return makeQuery({ ...state, selectFn: args[0], selectAll: undefined })
},

where(fn) {
Expand Down
18 changes: 18 additions & 0 deletions src/ch/subquery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,21 @@ describe("CTE shadowing", () => {
expect(compiled.tenantScope).toBe("cross-tenant")
})
})

describe("bare select() over a derived source", () => {
const inner = CH.from(Events)
.select(($) => ({ trace: $.TraceId, total: CH.count(), raw: CH.untypedExpr("1") }))
.groupBy("trace")

it("selects every alias of a subquery, typed ones decoding as before", () => {
const compiled = compileCHUnsafe(CH.fromQuery(inner, "w").select(), {})
expect(compiled.sql).toContain("SELECT\n w.trace AS trace,\n w.total AS total,\n w.raw AS raw")
expect(compiled.sql).toContain("FROM (")
})

it("selects every alias of a union", () => {
const branch = CH.from(Events).select(($) => ({ trace: $.TraceId }))
const { sql } = compileCHUnsafe(CH.fromUnion(CH.unionAll(branch, branch), "u").select(), {})
expect(sql).toContain("u.trace AS trace")
})
})
8 changes: 1 addition & 7 deletions tests/fuzz/known-issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,7 @@ const planIssues: ReadonlyArray<KnownIssue<PlanInput>> = [
target === "postgres" &&
[...nodes(plan)].some((node) => node.k === "value" && typeof node.v === "number" && !Number.isInteger(node.v)),
},
{
id: "fromquery-select-all",
reason:
"`CH.fromQuery(q, alias).select()` is typed as selecting every column, but a subquery source has no column " +
"list to expand, so compile dies with QueryBuilderDefect `select() is required`. `select(...names)` works.",
matches: ({ plan }) => plan.wrap === "all",
},

]

export const knownPlanIssue = (input: PlanInput): string | undefined => firstMatch(planIssues, input)
Loading