-
Notifications
You must be signed in to change notification settings - Fork 12
refactor(sql-ast): replace instanceof with kind discriminants #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 23 commits
1f944c9
947625c
6e7a1a4
5dab01e
bd01359
e86fc3d
9c4b253
2018719
4020602
a2505ae
1775c86
1378d55
51d1596
6f89648
ee6869a
f664f15
a166dcf
e7d3be1
f88d415
ea84c2f
ceda6cf
fcb3ae7
ed06563
c957135
7aedb4b
8ee4f5a
248e7a8
3c91436
e130fa4
a9d18ea
c1cb7e9
bb7a447
8b7ae37
7e71428
6e210df
f008607
9fd5b10
d736dbd
a46c196
6678a48
95ae31b
2dcb590
994d39d
059521a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |||||
| */ | ||||||
| import type { PlanRefs } from '@prisma-next/contract/types'; | ||||||
| import type { SqlContract, SqlStorage } from '@prisma-next/sql-contract/types'; | ||||||
| import { ColumnRef, type QueryAst, SelectAst } from '@prisma-next/sql-relational-core/ast'; | ||||||
| import type { AnyQueryAst, ColumnRef } from '@prisma-next/sql-relational-core/ast'; | ||||||
| import { ifDefined } from '@prisma-next/utils/defined'; | ||||||
| import { DeleteQueryNode, InsertQueryNode, SelectQueryNode, UpdateQueryNode } from 'kysely'; | ||||||
| import { KYSELY_TRANSFORM_ERROR_CODES, KyselyTransformError } from './errors'; | ||||||
|
|
@@ -21,7 +21,7 @@ import { transformSelect } from './transform-select'; | |||||
|
|
||||||
| export type { TransformResult }; | ||||||
|
|
||||||
| function extractRefsFromAst(ast: QueryAst): PlanRefs { | ||||||
| function extractRefsFromAst(ast: AnyQueryAst): PlanRefs { | ||||||
| return ast.collectRefs(); | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -49,7 +49,7 @@ export function transformKyselyToPnAst( | |||||
|
|
||||||
| const ctx = createContext(contract, parameters); | ||||||
|
|
||||||
| let ast: QueryAst; | ||||||
| let ast: AnyQueryAst; | ||||||
| if (SelectQueryNode.is(query)) { | ||||||
| ast = transformSelect(query, ctx); | ||||||
| } else if (InsertQueryNode.is(query)) { | ||||||
|
|
@@ -77,19 +77,21 @@ export function transformKyselyToPnAst( | |||||
|
|
||||||
| let projection: Record<string, string> | undefined; | ||||||
| let projectionTypes: Record<string, string> | undefined; | ||||||
| if (ast instanceof SelectAst) { | ||||||
| const select = ast.kind === 'select' ? ast : undefined; | ||||||
| if (select) { | ||||||
| projection = Object.fromEntries( | ||||||
| ast.projection.map((projected) => [ | ||||||
| projected.alias, | ||||||
| projected.expr instanceof ColumnRef ? projected.expr.column : projected.alias, | ||||||
| ]), | ||||||
| select.projection.map((projected) => { | ||||||
| const col = | ||||||
| projected.expr.kind === 'column-ref' ? (projected.expr as ColumnRef) : undefined; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| return [projected.alias, col?.column ?? projected.alias]; | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| projectionTypes = {}; | ||||||
| for (const projected of ast.projection) { | ||||||
| if (projected.expr instanceof ColumnRef) { | ||||||
| const column = | ||||||
| ctx.contract.storage.tables[projected.expr.table]?.columns[projected.expr.column]; | ||||||
| for (const projected of select.projection) { | ||||||
| const col = projected.expr.kind === 'column-ref' ? (projected.expr as ColumnRef) : undefined; | ||||||
|
aqrln marked this conversation as resolved.
Outdated
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if (col) { | ||||||
| const column = ctx.contract.storage.tables[col.table]?.columns[col.column]; | ||||||
| if (column) { | ||||||
| projectionTypes[projected.alias] = column.codecId; | ||||||
| } | ||||||
|
|
@@ -105,8 +107,8 @@ export function transformKyselyToPnAst( | |||||
| 'projectionTypes', | ||||||
| projectionTypes && Object.keys(projectionTypes).length > 0 ? projectionTypes : undefined, | ||||||
| ), | ||||||
| ...ifDefined('selectAllIntent', ast instanceof SelectAst ? ast.selectAllIntent : undefined), | ||||||
| ...ifDefined('limit', ast instanceof SelectAst ? ast.limit : undefined), | ||||||
| ...ifDefined('selectAllIntent', select?.selectAllIntent), | ||||||
| ...ifDefined('limit', select?.limit), | ||||||
| }; | ||||||
|
|
||||||
| return { ast, metaAdditions }; | ||||||
|
|
@@ -131,7 +133,7 @@ export function transformKyselyToPnAstCollectingParams( | |||||
|
|
||||||
| const ctx = createContext(contract); | ||||||
|
|
||||||
| let ast: QueryAst; | ||||||
| let ast: AnyQueryAst; | ||||||
| if (SelectQueryNode.is(query)) { | ||||||
| ast = transformSelect(query, ctx); | ||||||
| } else if (InsertQueryNode.is(query)) { | ||||||
|
|
@@ -159,19 +161,21 @@ export function transformKyselyToPnAstCollectingParams( | |||||
|
|
||||||
| let projection: Record<string, string> | undefined; | ||||||
| let projectionTypes: Record<string, string> | undefined; | ||||||
| if (ast instanceof SelectAst) { | ||||||
| const select = ast.kind === 'select' ? ast : undefined; | ||||||
| if (select) { | ||||||
| projection = Object.fromEntries( | ||||||
| ast.projection.map((projected) => [ | ||||||
| projected.alias, | ||||||
| projected.expr instanceof ColumnRef ? projected.expr.column : projected.alias, | ||||||
| ]), | ||||||
| select.projection.map((projected) => { | ||||||
| const col = | ||||||
| projected.expr.kind === 'column-ref' ? (projected.expr as ColumnRef) : undefined; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| return [projected.alias, col?.column ?? projected.alias]; | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| projectionTypes = {}; | ||||||
| for (const projected of ast.projection) { | ||||||
| if (projected.expr instanceof ColumnRef) { | ||||||
| const column = | ||||||
| ctx.contract.storage.tables[projected.expr.table]?.columns[projected.expr.column]; | ||||||
| for (const projected of select.projection) { | ||||||
| const col = projected.expr.kind === 'column-ref' ? (projected.expr as ColumnRef) : undefined; | ||||||
| if (col) { | ||||||
| const column = ctx.contract.storage.tables[col.table]?.columns[col.column]; | ||||||
| if (column) { | ||||||
| projectionTypes[projected.alias] = column.codecId; | ||||||
| } | ||||||
|
|
@@ -187,8 +191,8 @@ export function transformKyselyToPnAstCollectingParams( | |||||
| 'projectionTypes', | ||||||
| projectionTypes && Object.keys(projectionTypes).length > 0 ? projectionTypes : undefined, | ||||||
| ), | ||||||
| ...ifDefined('selectAllIntent', ast instanceof SelectAst ? ast.selectAllIntent : undefined), | ||||||
| ...ifDefined('limit', ast instanceof SelectAst ? ast.limit : undefined), | ||||||
| ...ifDefined('selectAllIntent', select?.selectAllIntent), | ||||||
| ...ifDefined('limit', select?.limit), | ||||||
| }; | ||||||
|
|
||||||
| return { ast, params: ctx.params, metaAdditions }; | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,7 @@ import type { SqlContract, SqlStorage } from '@prisma-next/sql-contract/types'; | |||||||||||||||||
| import { | ||||||||||||||||||
| type BoundWhereExpr, | ||||||||||||||||||
| ListLiteralExpr, | ||||||||||||||||||
| ParamRef, | ||||||||||||||||||
| SelectAst, | ||||||||||||||||||
| type SelectAst, | ||||||||||||||||||
| type ToWhereExpr, | ||||||||||||||||||
| } from '@prisma-next/sql-relational-core/ast'; | ||||||||||||||||||
| import type { CompiledQuery } from 'kysely'; | ||||||||||||||||||
|
|
@@ -29,21 +28,25 @@ export function buildKyselyWhereExpr<Row>( | |||||||||||||||||
| options: BuildKyselyPlanOptions = {}, | ||||||||||||||||||
| ): ToWhereExpr { | ||||||||||||||||||
| const plan = buildKyselyPlan(contract, compiledQuery, options); | ||||||||||||||||||
| if (!(plan.ast instanceof SelectAst) || !plan.ast.where) { | ||||||||||||||||||
| if (plan.ast.kind !== 'select') { | ||||||||||||||||||
| throw new Error('whereExpr(...) requires a select query with a where clause'); | ||||||||||||||||||
| } | ||||||||||||||||||
| const selectAst = plan.ast as SelectAst; | ||||||||||||||||||
| if (!selectAst.where) { | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
| throw new Error('whereExpr(...) requires a select query with a where clause'); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const indexes = [...new Set(collectParamIndexes(plan.ast.where))].sort((a, b) => a - b); | ||||||||||||||||||
| const indexes = [...new Set(collectParamIndexes(selectAst.where))].sort((a, b) => a - b); | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||
| if (indexes.length === 0) { | ||||||||||||||||||
| return new LaneWhereExpr({ | ||||||||||||||||||
| expr: plan.ast.where, | ||||||||||||||||||
| expr: selectAst.where, | ||||||||||||||||||
| params: [], | ||||||||||||||||||
| paramDescriptors: [], | ||||||||||||||||||
| }); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const remap = new Map<number, number>(indexes.map((index, i) => [index, i + 1])); | ||||||||||||||||||
| const remappedExpr = remapParamIndexes(plan.ast.where, remap); | ||||||||||||||||||
| const remappedExpr = remapParamIndexes(selectAst.where, remap); | ||||||||||||||||||
| const params = indexes.map((index) => { | ||||||||||||||||||
| if (index <= 0 || index > plan.params.length) { | ||||||||||||||||||
| throw new Error(`whereExpr(...) payload is invalid: missing param value for index ${index}`); | ||||||||||||||||||
|
|
@@ -97,7 +100,7 @@ function remapParamIndexes( | |||||||||||||||||
| listLiteral: (list) => | ||||||||||||||||||
| new ListLiteralExpr( | ||||||||||||||||||
| list.values.map((value) => { | ||||||||||||||||||
| if (!(value instanceof ParamRef)) { | ||||||||||||||||||
| if (value.kind !== 'param-ref') { | ||||||||||||||||||
| return value; | ||||||||||||||||||
| } | ||||||||||||||||||
| const newIndex = remap.get(value.index); | ||||||||||||||||||
|
|
||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
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.
This is not ideal. If
AnyWhereExpris expected here, thentransformWhereExprshould returnAnyWhereExpr.