feat: provide the client in the computed field context #2789
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { match, P } from 'ts-pattern'; | |
| import { AnyNullClass, DbNullClass, JsonNullClass } from '../../../common-types'; | ||
| import type { OrArray } from '../../../utils/type-utils'; | ||
| import { AggregateOperators, DELEGATE_JOINED_FIELD_PREFIX, LOGICAL_COMBINATORS } from '../../constants'; | ||
| import type { ClientContract } from '../../contract'; | ||
| import type { | ||
| BooleanFilter, | ||
| BytesFilter, | ||
|
|
@@ -36,13 +37,38 @@ import { | |
| tmpAlias, | ||
| } from '../../query-utils'; | ||
|
|
||
| /** | ||
| * Arguments for constructing a CRUD dialect: either the client executing the queries — schema | ||
| * and options are derived from it, and it's handed to computed field implementations — or a | ||
| * standalone schema/options pair for uses that have no client (e.g. output transformation). | ||
| */ | ||
| export type CrudDialectArgs<Schema extends SchemaDef> = | ||
| | [client: ClientContract<Schema>] | ||
| | [schema: Schema, options: ClientOptions<Schema>]; | ||
|
|
||
| export abstract class BaseCrudDialect<Schema extends SchemaDef> { | ||
| protected eb = expressionBuilder<any, any>(); | ||
|
|
||
| protected readonly schema: Schema; | ||
| protected readonly options: ClientOptions<Schema>; | ||
|
|
||
| /** | ||
| * The client executing the query. Unset only when the dialect was constructed from a | ||
| * standalone schema/options pair, in which case it cannot evaluate computed fields. | ||
| */ | ||
| protected readonly client: ClientContract<Schema> | undefined; | ||
|
|
||
| constructor(...args: CrudDialectArgs<Schema>) { | ||
| if (args.length === 1) { | ||
| const [client] = args; | ||
| this.client = client; | ||
| this.schema = client.$schema; | ||
| this.options = client.$options; | ||
| } else { | ||
| [this.schema, this.options] = args; | ||
| this.client = undefined; | ||
| } | ||
| } | ||
|
|
||
| // #region capability flags1 | ||
|
|
||
|
|
@@ -1660,9 +1686,12 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> { | |
| if (!computer) { | ||
| throw createConfigError(`Computed field "${field}" implementation not provided for model "${model}"`); | ||
| } | ||
| // every query issued through the ORM builds the dialect from a client, and a dialect | ||
| // built from a standalone schema/options pair never inlines computed fields | ||
| invariant(this.client, `computed field "${field}" of model "${model}" needs a client to be evaluated`); | ||
| // `computedArgs` is the query-time args object for a parameterized computed | ||
| // field (undefined otherwise); forwarded as the implementation's 3rd argument. | ||
| return computer(this.eb, { modelAlias, client: this.client }, computedArgs); | ||
|
Comment on lines
+1689
to
+1694
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 4 'fieldRef\(|computedFields|\.parens\(' packages/orm/src tests/e2e
rg -n -C 6 'authorId|computed.*where|where.*computed|\$setAuth' tests/e2eRepository: zenstackhq/zenstack Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
file="packages/orm/src/client/crud/dialects/base-dialect.ts"
printf '%s\n' '--- target implementation ---'
sed -n '1640,1710p' "$file"
printf '%s\n' '--- fieldRef declarations and call sites ---'
rg -n -C 5 'fieldRef' packages/orm/src --glob '*.ts' --glob '*.tsx' | head -n 240
printf '%s\n' '--- computed-field declarations and handlers ---'
rg -n -C 5 'computed|compute' packages/orm/src --glob '*.ts' --glob '*.tsx' | head -n 320
printf '%s\n' '--- parens API usage ---'
rg -n -C 3 '\.parens\(' packages/orm/src --glob '*.ts' --glob '*.tsx' | head -n 200Repository: zenstackhq/zenstack Length of output: 50375 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant test files ---'
rg -l -i 'computed|`@computed`|computedField' tests/e2e packages --glob '*.test.ts' --glob '*.ts' --glob '*.zmodel' | head -n 120
printf '%s\n' '--- PostgreSQL SQL generation and dialect tests ---'
rg -l -i 'postgres|toSql|compile|query.sql|sql' packages/orm/src tests/e2e/orm --glob '*test.ts' | head -n 120
printf '%s\n' '--- package metadata and available dependencies ---'
find packages/orm -maxdepth 2 -type f \( -name 'package.json' -o -name 'pnpm-lock.yaml' \) -print
rg -n '"kysely"|"pg"' package.json packages/*/package.json packages/orm/package.json 2>/dev/null | head -n 80Repository: zenstackhq/zenstack Length of output: 5817 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("packages/orm/src/client/crud/dialects/base-dialect.ts")
text = p.read_text()
for needle in ("fieldRef", "computedFields", "parens"):
print(f"--- {needle} ---")
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(i, line.strip())
PY
printf '%s\n' '--- exact target context ---'
nl -ba packages/orm/src/client/crud/dialects/base-dialect.ts | sed -n '1650,1705p'Repository: zenstackhq/zenstack Length of output: 4724 🏁 Script executed: #!/bin/bash
set -euo pipefail
set +e
node - <<'JS'
try {
const kysely = require('kysely');
console.log('kysely available');
const { PostgresAdapter, PostgresQueryCompiler, PostgresIntrospector, Kysely, sql } = kysely;
const db = new Kysely({
dialect: {
createAdapter: () => new PostgresAdapter(),
createDriver: () => { throw new Error('not used'); },
createIntrospector: (db) => new PostgresIntrospector(db),
createQueryCompiler: () => new PostgresQueryCompiler(),
},
});
const expression = sql`"authorId" = ${sql.val(2)}`;
const ungrouped = db.selectFrom('Post').selectAll().where(expression, '=', 3).compile();
const grouped = db.selectFrom('Post').selectAll().where(sql`(${expression})`, '=', 3).compile();
console.log(JSON.stringify({ ungrouped, grouped }, null, 2));
} catch (e) {
console.log(`${e.name}: ${e.message}`);
}
JS
set -eRepository: zenstackhq/zenstack Length of output: 236 🌐 Web query:
💡 Result: In Kysely, nested binary operation expressions and the use of parentheses are handled primarily through the ExpressionBuilder's parens method [1][2]. When building queries, the PostgresQueryCompiler uses a visitor pattern, including a visitBinaryOperation method, to compile operation nodes into SQL [3]. However, Kysely does not automatically wrap all nested binary operations in parentheses by default to avoid unnecessary SQL syntax [4]. To control operator precedence or force grouping in your generated SQL, you must explicitly use the eb.parens method provided by the ExpressionBuilder [1][5]. Key behaviors regarding parentheses in Kysely include: 1. Explicit Wrapping: You can use eb.parens to wrap binary expressions or other expressions [1][2]. For example: const result = await db.selectFrom('person').where((eb) => eb(eb.parens('age', '+', 1), '/', 100), '<', 0.1).execute; This generates: where ("age" + $1) / $2 < $3 [1][5]. 2. Expression Composition: Since Kysely expressions are composable, you can pass nested expressions directly into eb.parens to manage complex logic [1][6]. For logical operations (like.and or.or), Kysely often handles the necessary parentheses automatically when combining filter lists [7][5]. 3. Compiler Logic: The PostgresQueryCompiler (in default-query-compiler.ts) manages parent-node awareness to decide if specific query structures (like subqueries or CTEs) require wrapping, but for general binary expression trees, the structure is determined by how you build the expression tree using the builder [8][9]. If you find that your generated SQL is missing required parentheses for a specific complex operation, the recommended approach is to wrap the sub-expression explicitly using the.parens builder method rather than relying on automatic compiler behavior [1][4]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- computed-field tests ---'
sed -n '1,280p' tests/e2e/orm/client-api/computed-fields.test.ts
printf '%s\n' '--- PostgreSQL computed multischema tests ---'
sed -n '1,260p' tests/e2e/orm/client-api/pg-computed-multischema.test.ts
printf '%s\n' '--- computed-field type definitions ---'
sed -n '280,340p' packages/orm/src/client/options.ts
printf '%s\n' '--- package lock Kysely version ---'
rg -n -C 2 'kysely@|kysely:' pnpm-lock.yaml package.json packages/orm/package.json | head -n 100Repository: zenstackhq/zenstack Length of output: 19375 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- all direct computed handler definitions in tests and examples ---'
rg -n -C 8 'computedFields\s*[:=]|computedFields:' tests packages --glob '*.ts' --glob '*.tsx' --glob '*.js' | head -n 360
printf '%s\n' '--- expression-builder binary-expression implementation references ---'
rg -n 'eb\([^;]*[=<>!]|this\.eb\([^;]*[=<>!]|sql\.raw.*=' tests/e2e/orm/client-api/computed-fields.test.ts tests/e2e/orm packages/orm/test --glob '*.ts' | head -n 240Repository: zenstackhq/zenstack Length of output: 29985 🌐 Web query:
💡 Result: In the context of the Kysely query builder, BinaryOperationNode is a specific node in the library's internal abstract syntax tree (AST) used to represent binary expressions (e.g., leftOperand operator rightOperand) [1][2]. Because these internal nodes do not automatically enforce operator precedence or inject parentheses based on mathematical rules, the library relies on an explicit ParensNode to handle operator precedence [3]. Key technical details include: 1. Handling Precedence: Parentheses are not managed implicitly by the BinaryOperationNode itself. Instead, developers must explicitly wrap expressions using an expression builder method (e.g., eb.parens), which creates a ParensNode in the AST [3]. 2. AST Representation: When the query is compiled into SQL, the query compiler visits the nodes recursively [4][5]. If a ParensNode is encountered, the compiler explicitly appends opening and closing parentheses before and after visiting the nested node [4][5]. 3. Compilation Process: The visitBinaryOperation method in the query compiler performs a straightforward concatenation of the left operand, the operator, and the right operand [4][6]. It does not perform any logic to determine if parentheses are required for correct SQL precedence; that responsibility is shifted to the user or the query construction logic [3]. This design prioritizes explicit control over expression structure, ensuring that the generated SQL follows the intended precedence defined by the user during the construction of the query [3]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- existing boolean computed-field test ---'
sed -n '940,1045p' tests/e2e/orm/client-api/computed-fields.test.ts
printf '%s\n' '--- primitive boolean filter construction ---'
sed -n '640,710p' packages/orm/src/client/crud/dialects/base-dialect.ts
sed -n '1080,1130p' packages/orm/src/client/crud/dialects/base-dialect.ts
printf '%s\n' '--- current diff/stat and target-file history summary ---'
git diff --stat
git status --short
git log --oneline -8 -- packages/orm/src/client/crud/dialects/base-dialect.tsRepository: zenstackhq/zenstack Length of output: 8951 Parenthesize computed expressions in Kysely does not add parentheses around nested binary expressions. The existing boolean computed-field test avoids invalid SQL because its handler calls 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,33 @@ | ||
| import type { SchemaDef } from '@zenstackhq/schema'; | ||
| import { match } from 'ts-pattern'; | ||
| import type { ClientContract } from '../../contract'; | ||
| import type { ClientOptions } from '../../options'; | ||
| import type { BaseCrudDialect, CrudDialectArgs } from './base-dialect'; | ||
| import { MySqlCrudDialect } from './mysql'; | ||
| import { PostgresCrudDialect } from './postgresql'; | ||
| import { SqliteCrudDialect } from './sqlite'; | ||
|
|
||
| /** | ||
| * Creates a CRUD dialect for the client's provider. Schema and options are taken from the | ||
| * client, which is also handed to computed field implementations, so prefer this overload | ||
| * whenever a client is available. | ||
| */ | ||
| export function getCrudDialect<Schema extends SchemaDef>(client: ClientContract<Schema>): BaseCrudDialect<Schema>; | ||
|
|
||
| /** | ||
| * Creates a CRUD dialect from a standalone schema/options pair, for uses that have no client | ||
| * (e.g. output transformation). Such a dialect cannot evaluate computed fields. | ||
| */ | ||
| export function getCrudDialect<Schema extends SchemaDef>( | ||
| schema: Schema, | ||
| options: ClientOptions<Schema>, | ||
| ): BaseCrudDialect<Schema>; | ||
|
|
||
| export function getCrudDialect<Schema extends SchemaDef>(...args: CrudDialectArgs<Schema>): BaseCrudDialect<Schema> { | ||
| const schema = args.length === 1 ? args[0].$schema : args[0]; | ||
| return match(schema.provider.type) | ||
| .with('sqlite', () => new SqliteCrudDialect(...args)) | ||
| .with('postgresql', () => new PostgresCrudDialect(...args)) | ||
| .with('mysql', () => new MySqlCrudDialect(...args)) | ||
| .exhaustive(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we assert
this.clienthere? I believe it's always available in this path. If so, we can make theclientfield non-optional inComputedFieldContext, right? @evgenovalovThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I added the assert and made
clientrequired inComputedFieldContext.The dialect field itself stays optional, because
getCrudDialectis public and can still be called with schema/options. In our own code no such path reaches a computed field — neither the policy plugin nor the soft-delete plugin callsfieldRef, andResultProcessoronly transforms output values.