From e22e03b3b15880c96c3abd20b647c3d6b8f9a4f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20Koskim=C3=A4ki?= Date: Thu, 21 Jul 2022 13:36:33 +0300 Subject: [PATCH] cleanup --- package.json | 2 +- src/operation-node/create-table-node.ts | 5 +- src/operation-node/references-node.ts | 18 +++-- src/parser/on-commit-action-parse.ts | 12 ++++ src/parser/on-modify-action-parser.ts | 14 ++++ src/query-compiler/default-query-compiler.ts | 44 ++++++------ src/schema/column-definition-builder.ts | 5 +- src/schema/create-table-builder.ts | 3 +- src/schema/foreign-key-constraint-builder.ts | 5 +- test/node/src/sanitize-identifiers.test.ts | 74 ++++++++++++++++++++ test/node/src/select.test.ts | 34 +++++++++ 11 files changed, 183 insertions(+), 33 deletions(-) create mode 100644 src/parser/on-commit-action-parse.ts create mode 100644 src/parser/on-modify-action-parser.ts create mode 100644 test/node/src/sanitize-identifiers.test.ts diff --git a/package.json b/package.json index f830eb232..00852d5a0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "kysely", - "version": "0.19.11", + "version": "0.19.12", "description": "Type safe SQL query builder", "repository": { "type": "git", diff --git a/src/operation-node/create-table-node.ts b/src/operation-node/create-table-node.ts index eea3b6c78..9b8e9c363 100644 --- a/src/operation-node/create-table-node.ts +++ b/src/operation-node/create-table-node.ts @@ -3,8 +3,11 @@ import { OperationNode } from './operation-node.js' import { TableNode } from './table-node.js' import { ConstraintNode } from './constraint-node.js' import { ColumnDefinitionNode } from './column-definition-node.js' +import { ArrayItemType } from '../util/type-utils.js' + +export const ON_COMMIT_ACTIONS = ['preserve rows', 'delete rows', 'drop'] +export type OnCommitAction = ArrayItemType -export type OnCommitAction = 'preserve rows' | 'delete rows' | 'drop' export type CreateTableNodeParams = Omit< CreateTableNode, 'kind' | 'table' | 'columns' | 'constraints' diff --git a/src/operation-node/references-node.ts b/src/operation-node/references-node.ts index 8e61c4d89..b143e99fa 100644 --- a/src/operation-node/references-node.ts +++ b/src/operation-node/references-node.ts @@ -2,13 +2,19 @@ import { OperationNode } from './operation-node.js' import { ColumnNode } from './column-node.js' import { TableNode } from './table-node.js' import { freeze } from '../util/object-utils.js' +import { ArrayItemType } from '../util/type-utils.js' -export type OnModifyForeignAction = - | 'no action' - | 'restrict' - | 'cascade' - | 'set null' - | 'set default' +export const ON_MODIFY_FOREIGN_ACTIONS = [ + 'no action', + 'restrict', + 'cascade', + 'set null', + 'set default', +] as const + +export type OnModifyForeignAction = ArrayItemType< + typeof ON_MODIFY_FOREIGN_ACTIONS +> export interface ReferencesNode extends OperationNode { readonly kind: 'ReferencesNode' diff --git a/src/parser/on-commit-action-parse.ts b/src/parser/on-commit-action-parse.ts new file mode 100644 index 000000000..e510c67c6 --- /dev/null +++ b/src/parser/on-commit-action-parse.ts @@ -0,0 +1,12 @@ +import { + OnCommitAction, + ON_COMMIT_ACTIONS, +} from '../operation-node/create-table-node.js' + +export function parseOnCommitAction(action: OnCommitAction): OnCommitAction { + if (ON_COMMIT_ACTIONS.includes(action)) { + return action + } + + throw new Error(`invalid OnCommitAction ${action}`) +} diff --git a/src/parser/on-modify-action-parser.ts b/src/parser/on-modify-action-parser.ts new file mode 100644 index 000000000..3885f8b98 --- /dev/null +++ b/src/parser/on-modify-action-parser.ts @@ -0,0 +1,14 @@ +import { + OnModifyForeignAction, + ON_MODIFY_FOREIGN_ACTIONS, +} from '../operation-node/references-node.js' + +export function parseOnModifyForeignAction( + action: OnModifyForeignAction +): OnModifyForeignAction { + if (ON_MODIFY_FOREIGN_ACTIONS.includes(action)) { + return action + } + + throw new Error(`invalid OnModifyForeignAction ${action}`) +} diff --git a/src/query-compiler/default-query-compiler.ts b/src/query-compiler/default-query-compiler.ts index 0735b20e4..83ddad0a1 100644 --- a/src/query-compiler/default-query-compiler.ts +++ b/src/query-compiler/default-query-compiler.ts @@ -376,26 +376,6 @@ export class DefaultQueryCompiler this.append(this.sanitizeIdentifier(node.identifier)) } - protected sanitizeIdentifier = (identifier: string): string => { - const leftWrapper = this.getLeftIdentifierWrapper() - const rightWrapper = this.getRightIdentifierWrapper() - - let sanitizedIdentifier = identifier - .split('') - .map((char) => { - if (char === leftWrapper) { - return `${leftWrapper}${leftWrapper}` - } else if (char === rightWrapper) { - return `${rightWrapper}${rightWrapper}` - } - - return char - }) - .join('') - - return sanitizedIdentifier - } - protected override visitFilter(node: FilterNode): void { if (node.left) { this.visitNode(node.left) @@ -1161,6 +1141,30 @@ export class DefaultQueryCompiler return '$' + this.numParameters } + protected sanitizeIdentifier(identifier: string): string { + if (!isString(identifier)) { + throw new Error( + 'a non-string identifier was passed to sanitizeIdentifier.' + ) + } + + const leftWrap = this.getLeftIdentifierWrapper() + const rightWrap = this.getRightIdentifierWrapper() + + let sanitized = '' + for (const c of identifier) { + sanitized += c + + if (c === leftWrap) { + sanitized += leftWrap + } else if (c === rightWrap) { + sanitized += rightWrap + } + } + + return sanitized + } + protected addParameter(parameter: unknown): void { this.#parameters.push(parameter) } diff --git a/src/schema/column-definition-builder.ts b/src/schema/column-definition-builder.ts index 29234dcf8..a73314ca4 100644 --- a/src/schema/column-definition-builder.ts +++ b/src/schema/column-definition-builder.ts @@ -16,6 +16,7 @@ import { } from '../parser/default-value-parser.js' import { GeneratedNode } from '../operation-node/generated-node.js' import { DefaultValueNode } from '../operation-node/default-value-node.js' +import { parseOnModifyForeignAction } from '../parser/on-modify-action-parser.js' export interface ColumnDefinitionBuilderInterface { /** @@ -236,7 +237,7 @@ export class ColumnDefinitionBuilder ColumnDefinitionNode.cloneWith(this.#node, { references: ReferencesNode.cloneWithOnDelete( this.#node.references, - onDelete + parseOnModifyForeignAction(onDelete) ), }) ) @@ -251,7 +252,7 @@ export class ColumnDefinitionBuilder ColumnDefinitionNode.cloneWith(this.#node, { references: ReferencesNode.cloneWithOnUpdate( this.#node.references, - onUpdate + parseOnModifyForeignAction(onUpdate) ), }) ) diff --git a/src/schema/create-table-builder.ts b/src/schema/create-table-builder.ts index 3d1f1b605..e9b27bdf6 100644 --- a/src/schema/create-table-builder.ts +++ b/src/schema/create-table-builder.ts @@ -23,6 +23,7 @@ import { UniqueConstraintNode } from '../operation-node/unique-constraint-node.j import { CheckConstraintNode } from '../operation-node/check-constraint-node.js' import { AnyRawBuilder } from '../util/type-utils.js' import { parseTable } from '../parser/table-parser.js' +import { parseOnCommitAction } from '../parser/on-commit-action-parse.js' /** * This builder can be used to create a `create table` query. @@ -60,7 +61,7 @@ export class CreateTableBuilder return new CreateTableBuilder({ ...this.#props, createTableNode: CreateTableNode.cloneWith(this.#props.createTableNode, { - onCommit, + onCommit: parseOnCommitAction(onCommit), }), }) } diff --git a/src/schema/foreign-key-constraint-builder.ts b/src/schema/foreign-key-constraint-builder.ts index fc1bf2382..ee09bd906 100644 --- a/src/schema/foreign-key-constraint-builder.ts +++ b/src/schema/foreign-key-constraint-builder.ts @@ -1,6 +1,7 @@ import { ForeignKeyConstraintNode } from '../operation-node/foreign-key-constraint-node.js' import { OperationNodeSource } from '../operation-node/operation-node-source.js' import { OnModifyForeignAction } from '../operation-node/references-node.js' +import { parseOnModifyForeignAction } from '../parser/on-modify-action-parser.js' import { preventAwait } from '../util/prevent-await.js' export interface ForeignKeyConstraintBuilderInterface { @@ -22,7 +23,7 @@ export class ForeignKeyConstraintBuilder onDelete(onDelete: OnModifyForeignAction): ForeignKeyConstraintBuilder { return new ForeignKeyConstraintBuilder( ForeignKeyConstraintNode.cloneWith(this.#node, { - onDelete, + onDelete: parseOnModifyForeignAction(onDelete), }) ) } @@ -30,7 +31,7 @@ export class ForeignKeyConstraintBuilder onUpdate(onUpdate: OnModifyForeignAction): ForeignKeyConstraintBuilder { return new ForeignKeyConstraintBuilder( ForeignKeyConstraintNode.cloneWith(this.#node, { - onUpdate, + onUpdate: parseOnModifyForeignAction(onUpdate), }) ) } diff --git a/test/node/src/sanitize-identifiers.test.ts b/test/node/src/sanitize-identifiers.test.ts new file mode 100644 index 000000000..1239e8bc0 --- /dev/null +++ b/test/node/src/sanitize-identifiers.test.ts @@ -0,0 +1,74 @@ +import { Updateable } from '../../../dist/cjs' + +import { + BUILT_IN_DIALECTS, + destroyTest, + initTest, + TestContext, + Person, + testSql, +} from './test-setup.js' + +for (const dialect of BUILT_IN_DIALECTS) { + describe(`${dialect}: sanitize identifiers`, () => { + let ctx: TestContext + + before(async function () { + ctx = await initTest(this, dialect) + }) + + after(async () => { + await destroyTest(ctx) + }) + + it('should escape identifier quotes', async () => { + const obj: Record = { + first_name: 'foo', + 'last_name"`': 'bar', + } + + const person = obj as unknown as Updateable + const query = ctx.db.updateTable('person').set(person) + + testSql(query, dialect, { + postgres: { + sql: 'update "person" set "first_name" = $1, "last_name""`" = $2', + parameters: ['foo', 'bar'], + }, + mysql: { + sql: 'update `person` set `first_name` = ?, `last_name"``` = ?', + parameters: ['foo', 'bar'], + }, + sqlite: { + sql: 'update "person" set "first_name" = ?, "last_name""`" = ?', + parameters: ['foo', 'bar'], + }, + }) + }) + + it('should escape multiple identifier quotes', async () => { + const obj: Record = { + first_name: 'foo', + 'last_name""``': 'bar', + } + + const person = obj as unknown as Updateable + const query = ctx.db.updateTable('person').set(person) + + testSql(query, dialect, { + postgres: { + sql: 'update "person" set "first_name" = $1, "last_name""""``" = $2', + parameters: ['foo', 'bar'], + }, + mysql: { + sql: 'update `person` set `first_name` = ?, `last_name""````` = ?', + parameters: ['foo', 'bar'], + }, + sqlite: { + sql: 'update "person" set "first_name" = ?, "last_name""""``" = ?', + parameters: ['foo', 'bar'], + }, + }) + }) + }) +} diff --git a/test/node/src/select.test.ts b/test/node/src/select.test.ts index 6afe8af29..0aa96fd92 100644 --- a/test/node/src/select.test.ts +++ b/test/node/src/select.test.ts @@ -640,5 +640,39 @@ for (const dialect of BUILT_IN_DIALECTS) { }) } } + + it.skip('perf', async () => { + const ids = Array.from({ length: 100 }).map(() => + Math.round(Math.random() * 1000) + ) + + function test() { + return ctx.db + .updateTable('person') + .set({ + first_name: 'foo', + last_name: 'bar', + id: 100, + gender: 'other', + }) + .where('id', 'in', ids) + .compile() + } + + // Warmup + for (let i = 0; i < 1000; ++i) { + test() + } + + const time = Date.now() + const N = 100000 + + for (let i = 0; i < N; ++i) { + test() + } + + const endTime = Date.now() + console.log((endTime - time) / N) + }) }) }