From 129ecc016701eb8956b1a4177c4c43585257d853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20Koskim=C3=A4ki?= Date: Wed, 24 Nov 2021 13:01:46 +0200 Subject: [PATCH] code cleanup --- src/operation-node/create-schema-node.ts | 6 +-- src/operation-node/create-table-node.ts | 53 +------------------ src/operation-node/on-duplicate-key-node.ts | 1 - src/operation-node/operation-node-source.ts | 4 +- .../operation-node-transformer.ts | 3 +- src/query-compiler/default-query-compiler.ts | 2 +- src/schema/create-schema-builder.ts | 4 +- src/schema/create-table-builder.ts | 20 +++---- test/src/select.test.ts | 27 ++++++++++ test/src/test-setup.ts | 7 ++- 10 files changed, 55 insertions(+), 72 deletions(-) diff --git a/src/operation-node/create-schema-node.ts b/src/operation-node/create-schema-node.ts index efdf05a51..38f94c7b7 100644 --- a/src/operation-node/create-schema-node.ts +++ b/src/operation-node/create-schema-node.ts @@ -30,13 +30,13 @@ export const CreateSchemaNode = freeze({ }) }, - cloneWithModifier( + cloneWith( createSchema: CreateSchemaNode, - modifier: CreateSchemaNodeModifier + params: CreateSchemaNodeParams ): CreateSchemaNode { return freeze({ ...createSchema, - modifier, + ...params, }) }, }) diff --git a/src/operation-node/create-table-node.ts b/src/operation-node/create-table-node.ts index 5718807db..144423ee2 100644 --- a/src/operation-node/create-table-node.ts +++ b/src/operation-node/create-table-node.ts @@ -1,10 +1,6 @@ import { freeze } from '../util/object-utils.js' -import { CheckConstraintNode } from './check-constraint-node.js' import { OperationNode } from './operation-node.js' import { TableNode } from './table-node.js' -import { PrimaryConstraintNode } from './primary-constraint-node.js' -import { UniqueConstraintNode } from './unique-constraint-node.js' -import { ForeignKeyConstraintNode } from './foreign-key-constraint-node.js' import { ConstraintNode } from './constraint-node.js' import { ColumnDefinitionNode } from './column-definition-node.js' @@ -54,54 +50,9 @@ export const CreateTableNode = freeze({ }) }, - cloneWithPrimaryKeyConstraint( + cloneWithConstraint( createTable: CreateTableNode, - constraintName: string, - columns: string[] - ): CreateTableNode { - const constraint = PrimaryConstraintNode.create(columns, constraintName) - - return freeze({ - ...createTable, - constraints: createTable.constraints - ? freeze([...createTable.constraints, constraint]) - : freeze([constraint]), - }) - }, - - cloneWithUniqueConstraint( - createTable: CreateTableNode, - constraintName: string, - columns: string[] - ): CreateTableNode { - const constraint = UniqueConstraintNode.create(columns, constraintName) - - return freeze({ - ...createTable, - constraints: createTable.constraints - ? freeze([...createTable.constraints, constraint]) - : freeze([constraint]), - }) - }, - - cloneWithCheckConstraint( - createTable: CreateTableNode, - constraintName: string, - sql: string - ): CreateTableNode { - const constraint = CheckConstraintNode.create(sql, constraintName) - - return freeze({ - ...createTable, - constraints: createTable.constraints - ? freeze([...createTable.constraints, constraint]) - : freeze([constraint]), - }) - }, - - cloneWithForeignKeyConstraint( - createTable: CreateTableNode, - constraint: ForeignKeyConstraintNode + constraint: ConstraintNode ): CreateTableNode { return freeze({ ...createTable, diff --git a/src/operation-node/on-duplicate-key-node.ts b/src/operation-node/on-duplicate-key-node.ts index 70d4032d8..b6e8fb949 100644 --- a/src/operation-node/on-duplicate-key-node.ts +++ b/src/operation-node/on-duplicate-key-node.ts @@ -1,5 +1,4 @@ import { freeze } from '../util/object-utils.js' -import { ColumnNode } from './column-node.js' import { ColumnUpdateNode } from './column-update-node.js' import { OperationNode } from './operation-node.js' diff --git a/src/operation-node/operation-node-source.ts b/src/operation-node/operation-node-source.ts index b651fa9c4..abee522dc 100644 --- a/src/operation-node/operation-node-source.ts +++ b/src/operation-node/operation-node-source.ts @@ -5,6 +5,8 @@ export interface OperationNodeSource { toOperationNode(): OperationNode } -export function isOperationNodeSource(obj: any): obj is OperationNodeSource { +export function isOperationNodeSource( + obj: unknown +): obj is OperationNodeSource { return isObject(obj) && isFunction(obj.toOperationNode) } diff --git a/src/operation-node/operation-node-transformer.ts b/src/operation-node/operation-node-transformer.ts index cc231068e..818689543 100644 --- a/src/operation-node/operation-node-transformer.ts +++ b/src/operation-node/operation-node-transformer.ts @@ -583,11 +583,12 @@ export class OperationNodeTransformer { kind: 'AlterTableNode', table: this.transformNode(node.table), renameTo: this.transformNode(node.renameTo), - renameColumn: this.transformNode(node.renameColumn), setSchema: this.transformNode(node.setSchema), + renameColumn: this.transformNode(node.renameColumn), addColumn: this.transformNode(node.addColumn), dropColumn: this.transformNode(node.dropColumn), alterColumn: this.transformNode(node.alterColumn), + modifyColumn: this.transformNode(node.modifyColumn), addConstraint: this.transformNode(node.addConstraint), dropConstraint: this.transformNode(node.dropConstraint), } diff --git a/src/query-compiler/default-query-compiler.ts b/src/query-compiler/default-query-compiler.ts index 35616e20e..52213bbde 100644 --- a/src/query-compiler/default-query-compiler.ts +++ b/src/query-compiler/default-query-compiler.ts @@ -930,7 +930,7 @@ const SELECT_MODIFIER_SQL: Record = { ForNoKeyUpdate: 'for no key update', ForUpdate: 'for update', ForShare: 'for share', - NoWait: 'no wait', + NoWait: 'nowait', SkipLocked: 'skip locked', } diff --git a/src/schema/create-schema-builder.ts b/src/schema/create-schema-builder.ts index 65bb4d2a6..5964c0b75 100644 --- a/src/schema/create-schema-builder.ts +++ b/src/schema/create-schema-builder.ts @@ -17,9 +17,9 @@ export class CreateSchemaBuilder implements OperationNodeSource, Compilable { ifNotExists(): CreateSchemaBuilder { return new CreateSchemaBuilder({ ...this.#props, - createSchemaNode: CreateSchemaNode.cloneWithModifier( + createSchemaNode: CreateSchemaNode.cloneWith( this.#props.createSchemaNode, - 'IfNotExists' + { modifier: 'IfNotExists' } ), }) } diff --git a/src/schema/create-table-builder.ts b/src/schema/create-table-builder.ts index 0af26195e..a0cc9e2cb 100644 --- a/src/schema/create-table-builder.ts +++ b/src/schema/create-table-builder.ts @@ -16,6 +16,9 @@ import { DataTypeExpression, parseDataTypeExpression, } from '../parser/data-type-parser.js' +import { PrimaryConstraintNode } from '../operation-node/primary-constraint-node.js' +import { UniqueConstraintNode } from '../operation-node/unique-constraint-node.js' +import { CheckConstraintNode } from '../operation-node/check-constraint-node.js' /** * This builder can be used to create a `create table` query. @@ -128,10 +131,9 @@ export class CreateTableBuilder ): CreateTableBuilder { return new CreateTableBuilder({ ...this.#props, - createTableNode: CreateTableNode.cloneWithPrimaryKeyConstraint( + createTableNode: CreateTableNode.cloneWithConstraint( this.#props.createTableNode, - constraintName, - columns + PrimaryConstraintNode.create(columns, constraintName) ), }) } @@ -153,10 +155,9 @@ export class CreateTableBuilder ): CreateTableBuilder { return new CreateTableBuilder({ ...this.#props, - createTableNode: CreateTableNode.cloneWithUniqueConstraint( + createTableNode: CreateTableNode.cloneWithConstraint( this.#props.createTableNode, - constraintName, - columns + UniqueConstraintNode.create(columns, constraintName) ), }) } @@ -178,10 +179,9 @@ export class CreateTableBuilder ): CreateTableBuilder { return new CreateTableBuilder({ ...this.#props, - createTableNode: CreateTableNode.cloneWithCheckConstraint( + createTableNode: CreateTableNode.cloneWithConstraint( this.#props.createTableNode, - constraintName, - checkExpression + CheckConstraintNode.create(checkExpression, constraintName) ), }) } @@ -233,7 +233,7 @@ export class CreateTableBuilder return new CreateTableBuilder({ ...this.#props, - createTableNode: CreateTableNode.cloneWithForeignKeyConstraint( + createTableNode: CreateTableNode.cloneWithConstraint( this.#props.createTableNode, builder.toOperationNode() ), diff --git a/test/src/select.test.ts b/test/src/select.test.ts index d71b55210..9d32130b7 100644 --- a/test/src/select.test.ts +++ b/test/src/select.test.ts @@ -405,6 +405,33 @@ for (const dialect of BUILT_IN_DIALECTS) { { first_name: 'Arnold' }, ]) }) + + for (const [methods, sql] of [ + [['forUpdate'], 'for update'], + [['forShare'], 'for share'], + [['forNoKeyUpdate'], 'for no key update'], + [['forKeyShare'], 'for key share'], + [['forUpdate', 'noWait'], 'for update nowait'], + [['forUpdate', 'skipLocked'], 'for update skip locked'], + ] as const) { + it(`should support "${sql}"`, async () => { + let query = ctx.db.selectFrom('person').selectAll() + + for (const method of methods) { + query = query[method]() + } + + testSql(query, dialect, { + postgres: { + sql: `select * from "person" ${sql}`, + parameters: [], + }, + mysql: NOT_SUPPORTED, + }) + + await query.execute() + }) + } } it('should use an aggregate function in a select call', async () => { diff --git a/test/src/test-setup.ts b/test/src/test-setup.ts index 8261eeea1..2884851ad 100644 --- a/test/src/test-setup.ts +++ b/test/src/test-setup.ts @@ -74,7 +74,10 @@ const PLUGINS: KyselyPlugin[] = [] if (process.env.TEST_TRANSFORMER) { console.log('running tests with a transformer') - PLUGINS.push(createNoopPlugin()) + // Add a noop transformer using a plugin to make sure that the + // OperationNodeTransformer base class is implemented correctly + // and all nodes and properties get cloned by default. + PLUGINS.push(createNoopTransformerPlugin()) } const DB_CONFIGS: PerDialect = { @@ -307,7 +310,7 @@ function getIdFromInsertResult(result: any): T { } } -function createNoopPlugin(): KyselyPlugin { +function createNoopTransformerPlugin(): KyselyPlugin { const transformer = new OperationNodeTransformer() return {