Skip to content

Commit

Permalink
add numInsertedOrUpdatedRows to InsertResult. (#188)
Browse files Browse the repository at this point in the history
* add numAffectedRows to InsertResult.

* update numUpdatedOrDeletedRows ts doc.

* update InsertQueryBuilder's execute().

* fix MysqlDriver not passing affected rows in result.

* make PostgresDriver pass row count in result of insert command.

* rename numAffectedRows to numInsertedOrUpdatedRows to align with others.

* add some result expectations @ node insert test.

* more node test stuff.

* add ts doc paragraph for numInsertedOrUpdatedRows.

* remove silly length assertion.

* add numUpdatedOrDeletedRows deprecation warning @ QueryExecutorBase.

* add numAffectedRows to QueryResult.

* add numAffectedRows (/w backwards compat) handling at builders.

* add numAffectedRows (/w backwards compat) handling @ dialect drivers.

* add logOnce helper fn.

* add logOnce unit test.

* nullish coalescing assignment not supported in node.js 14.
igalklebanov authored Dec 10, 2022
1 parent 458b3c0 commit 606b001
Showing 15 changed files with 301 additions and 43 deletions.
12 changes: 8 additions & 4 deletions src/dialect/mysql/mysql-driver.ts
Original file line number Diff line number Diff line change
@@ -121,17 +121,21 @@ class MysqlConnection implements DatabaseConnection {
if (isOkPacket(result)) {
const { insertId, affectedRows } = result

const numAffectedRows =
affectedRows !== undefined && affectedRows !== null
? BigInt(affectedRows)
: undefined

return {
insertId:
insertId !== undefined &&
insertId !== null &&
insertId.toString() !== '0'
? BigInt(insertId)
: undefined,
numUpdatedOrDeletedRows:
affectedRows !== undefined && insertId !== null
? BigInt(affectedRows)
: undefined,
// TODO: remove.
numUpdatedOrDeletedRows: numAffectedRows,
numAffectedRows,
rows: [],
}
} else if (Array.isArray(result)) {
12 changes: 10 additions & 2 deletions src/dialect/postgres/postgres-driver.ts
Original file line number Diff line number Diff line change
@@ -106,9 +106,17 @@ class PostgresConnection implements DatabaseConnection {
...compiledQuery.parameters,
])

if (result.command === 'UPDATE' || result.command === 'DELETE') {
if (
result.command === 'INSERT' ||
result.command === 'UPDATE' ||
result.command === 'DELETE'
) {
const numAffectedRows = BigInt(result.rowCount)

return {
numUpdatedOrDeletedRows: BigInt(result.rowCount),
// TODO: remove.
numUpdatedOrDeletedRows: numAffectedRows,
numAffectedRows,
rows: result.rows ?? [],
}
}
10 changes: 6 additions & 4 deletions src/dialect/sqlite/sqlite-driver.ts
Original file line number Diff line number Diff line change
@@ -76,11 +76,13 @@ class SqliteConnection implements DatabaseConnection {
} else {
const { changes, lastInsertRowid } = stmt.run(parameters)

const numAffectedRows =
changes !== undefined && changes !== null ? BigInt(changes) : undefined

return Promise.resolve({
numUpdatedOrDeletedRows:
changes !== undefined && changes !== null
? BigInt(changes)
: undefined,
// TODO: remove.
numUpdatedOrDeletedRows: numAffectedRows,
numAffectedRows,
insertId:
lastInsertRowid !== undefined && lastInsertRowid !== null
? BigInt(lastInsertRowid)
10 changes: 8 additions & 2 deletions src/driver/database-connection.ts
Original file line number Diff line number Diff line change
@@ -15,11 +15,17 @@ export interface DatabaseConnection {

export interface QueryResult<O> {
/**
* This is defined for update and delete queries and contains
* the number of rows the query updated/deleted.
* @deprecated use {@link QueryResult.numAffectedRows} instead.
*/
// TODO: remove.
readonly numUpdatedOrDeletedRows?: bigint

/**
* This is defined for insert, update and delete queries and contains
* the number of rows the query inserted/updated/deleted.
*/
readonly numAffectedRows?: bigint

/**
* This is defined for insert queries on dialects that return
* the auto incrementing primary key from an insert.
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -190,6 +190,7 @@ export {
UnknownRow,
} from './util/type-utils.js'
export * from './util/infer-result.js'
export { logOnce } from './util/log-once.js'

export {
SelectExpression,
9 changes: 7 additions & 2 deletions src/query-builder/delete-query-builder.ts
Original file line number Diff line number Diff line change
@@ -608,9 +608,14 @@ export class DeleteQueryBuilder<DB, TB extends keyof DB, O>

if (this.#props.executor.adapter.supportsReturning && query.returning) {
return result.rows
} else {
return [new DeleteResult(result.numUpdatedOrDeletedRows!) as unknown as O]
}

return [
new DeleteResult(
// TODO: remove numUpdatedOrDeletedRows.
(result.numAffectedRows ?? result.numUpdatedOrDeletedRows)!
) as any,
]
}

/**
10 changes: 8 additions & 2 deletions src/query-builder/insert-query-builder.ts
Original file line number Diff line number Diff line change
@@ -652,9 +652,15 @@ export class InsertQueryBuilder<DB, TB extends keyof DB, O>

if (this.#props.executor.adapter.supportsReturning && query.returning) {
return result.rows
} else {
return [new InsertResult(result.insertId) as unknown as O]
}

return [
new InsertResult(
result.insertId,
// TODO: remove numUpdatedOrDeletedRows.
result.numAffectedRows ?? result.numUpdatedOrDeletedRows
) as any,
]
}

/**
17 changes: 16 additions & 1 deletion src/query-builder/insert-result.ts
Original file line number Diff line number Diff line change
@@ -7,6 +7,9 @@
* need to use {@link ReturningInterface.returning} or {@link ReturningInterface.returningAll}
* to get out the inserted id.
*
* {@link numInsertedOrUpdatedRows} holds the number of (actually) inserted rows.
* On MySQL, updated rows are counted twice when using `on duplicate key update`.
*
* ### Examples
*
* ```ts
@@ -20,9 +23,14 @@
*/
export class InsertResult {
readonly #insertId: bigint | undefined
readonly #numInsertedOrUpdatedRows: bigint | undefined

constructor(insertId: bigint | undefined) {
constructor(
insertId: bigint | undefined,
numInsertedOrUpdatedRows: bigint | undefined
) {
this.#insertId = insertId
this.#numInsertedOrUpdatedRows = numInsertedOrUpdatedRows
}

/**
@@ -31,4 +39,11 @@ export class InsertResult {
get insertId(): bigint | undefined {
return this.#insertId
}

/**
* Affected rows count.
*/
get numInsertedOrUpdatedRows(): bigint | undefined {
return this.#numInsertedOrUpdatedRows
}
}
9 changes: 7 additions & 2 deletions src/query-builder/update-query-builder.ts
Original file line number Diff line number Diff line change
@@ -704,9 +704,14 @@ export class UpdateQueryBuilder<DB, UT extends keyof DB, TB extends keyof DB, O>

if (this.#props.executor.adapter.supportsReturning && query.returning) {
return result.rows
} else {
return [new UpdateResult(result.numUpdatedOrDeletedRows!) as unknown as O]
}

return [
new UpdateResult(
// TODO: remove numUpdatedOrDeletedRows.
(result.numAffectedRows ?? result.numUpdatedOrDeletedRows)!
) as any,
]
}

/**
30 changes: 29 additions & 1 deletion src/query-executor/query-executor-base.ts
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ import { QueryId } from '../util/query-id.js'
import { DialectAdapter } from '../dialect/dialect-adapter.js'
import { QueryExecutor } from './query-executor.js'
import { Deferred } from '../util/deferred.js'
import { logOnce } from '../util/log-once.js'

const NO_PLUGINS: ReadonlyArray<KyselyPlugin> = freeze([])

@@ -65,7 +66,13 @@ export abstract class QueryExecutorBase implements QueryExecutor {
): Promise<QueryResult<R>> {
return await this.provideConnection(async (connection) => {
const result = await connection.executeQuery(compiledQuery)
return this.#transformResult(result, queryId)

const transformedResult = await this.#transformResult(result, queryId)

// TODO: remove.
warnOfOutdatedDriverOrPlugins(result, transformedResult)

return transformedResult as any
})
}

@@ -118,3 +125,24 @@ export abstract class QueryExecutorBase implements QueryExecutor {
return result
}
}

// TODO: remove.
function warnOfOutdatedDriverOrPlugins(
result: QueryResult<unknown>,
transformedResult: QueryResult<unknown>
): void {
const { numAffectedRows } = result

if (
(numAffectedRows === undefined &&
result.numUpdatedOrDeletedRows === undefined) ||
(numAffectedRows !== undefined &&
transformedResult.numAffectedRows !== undefined)
) {
return
}

logOnce(
'kysely:warning: outdated driver/plugin detected! QueryResult.numUpdatedOrDeletedRows is deprecated and will be removed in a future release.'
)
}
14 changes: 14 additions & 0 deletions src/util/log-once.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const LOGGED_MESSAGES: Set<string> = new Set()

/**
* Use for system-level logging, such as deprecation messages.
* Logs a message and ensures it won't be logged again.
*/
export function logOnce(message: string): void {
if (LOGGED_MESSAGES.has(message)) {
return
}

LOGGED_MESSAGES.add(message)
console.log(message)
}
5 changes: 4 additions & 1 deletion test/node/src/delete.test.ts
Original file line number Diff line number Diff line change
@@ -104,7 +104,10 @@ for (const dialect of BUILT_IN_DIALECTS) {
sqlite: NOT_SUPPORTED,
})

await query.execute()
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(DeleteResult)
expect(result.numDeletedRows).to.equal(2n)
})
}

137 changes: 122 additions & 15 deletions test/node/src/insert.test.ts
Original file line number Diff line number Diff line change
@@ -58,9 +58,10 @@ for (const dialect of BUILT_IN_DIALECTS) {

const result = await query.executeTakeFirst()
expect(result).to.be.instanceOf(InsertResult)
expect(result.numInsertedOrUpdatedRows).to.equal(1n)

if (dialect === 'postgres') {
expect(result.insertId).to.equal(undefined)
expect(result.insertId).to.be.undefined
} else {
expect(result.insertId).to.be.a('bigint')
}
@@ -100,6 +101,7 @@ for (const dialect of BUILT_IN_DIALECTS) {

const result = await query.executeTakeFirst()
expect(result).to.be.instanceOf(InsertResult)
expect(result.numInsertedOrUpdatedRows).to.equal(1n)

expect(await getNewestPerson(ctx.db)).to.eql({
first_name: 'Hammo',
@@ -130,7 +132,15 @@ for (const dialect of BUILT_IN_DIALECTS) {
},
})

await query.execute()
const result = await query.executeTakeFirst()
expect(result).to.be.instanceOf(InsertResult)

const { pet_count } = await ctx.db
.selectFrom('pet')
.select(sql<string | number | bigint>`count(*)`.as('pet_count'))
.executeTakeFirstOrThrow()

expect(result.numInsertedOrUpdatedRows).to.equal(BigInt(pet_count))

const persons = await ctx.db
.selectFrom('person')
@@ -177,8 +187,13 @@ for (const dialect of BUILT_IN_DIALECTS) {
sqlite: NOT_SUPPORTED,
})

const res = await query.execute()
expect(res).to.have.length(2)
const result = await query.execute()

expect(result).to.have.length(2)
expect(result).to.deep.equal([
{ first_name: '1', gender: 'foo' },
{ first_name: '2', gender: 'bar' },
])
})
}

@@ -205,7 +220,16 @@ for (const dialect of BUILT_IN_DIALECTS) {
},
})

await query.execute()
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.numInsertedOrUpdatedRows).to.equal(1n)

if (dialect === 'postgres') {
expect(result.insertId).to.be.undefined
} else {
expect(result.insertId).to.be.a('bigint')
}
})

if (dialect === 'mysql') {
@@ -234,7 +258,8 @@ for (const dialect of BUILT_IN_DIALECTS) {
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.insertId).to.equal(undefined)
expect(result.insertId).to.be.undefined
expect(result.numInsertedOrUpdatedRows).to.equal(0n)
})
}

@@ -272,13 +297,15 @@ for (const dialect of BUILT_IN_DIALECTS) {
})

const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.numInsertedOrUpdatedRows).to.equal(0n)

if (dialect === 'sqlite') {
// SQLite seems to return the last inserted id even if nothing got inserted.
expect(result.insertId! > 0n).to.be.equal(true)
} else {
expect(result.insertId).to.equal(undefined)
expect(result.insertId).to.be.undefined
}
})
}
@@ -310,8 +337,10 @@ for (const dialect of BUILT_IN_DIALECTS) {
})

const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.insertId).to.equal(undefined)
expect(result.insertId).to.be.undefined
expect(result.numInsertedOrUpdatedRows).to.equal(0n)
})
}

@@ -342,7 +371,11 @@ for (const dialect of BUILT_IN_DIALECTS) {
sqlite: NOT_SUPPORTED,
})

await query.execute()
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.insertId).to.equal(BigInt(id))
expect(result.numInsertedOrUpdatedRows).to.equal(2n)

const updatedPet = await ctx.db
.selectFrom('pet')
@@ -394,7 +427,16 @@ for (const dialect of BUILT_IN_DIALECTS) {
},
})

await query.execute()
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.numInsertedOrUpdatedRows).to.equal(1n)

if (dialect === 'postgres') {
expect(result.insertId).to.be.undefined
} else {
expect(result.insertId).to.be.a('bigint')
}

const updatedPet = await ctx.db
.selectFrom('pet')
@@ -485,12 +527,71 @@ for (const dialect of BUILT_IN_DIALECTS) {
sqlite: NOT_SUPPORTED,
})

await query.execute()
const result = await query.execute()

expect(result).to.have.length(1)
expect(result[0]).to.containSubset({
species: 'hamster',
name: 'Catto',
})
})
}

it('should insert multiple rows', async () => {
const query = ctx.db.insertInto('person').values([
{
first_name: 'Foo',
last_name: 'Bar',
gender: 'other',
},
{
first_name: 'Baz',
last_name: 'Spam',
gender: 'other',
},
])

testSql(query, dialect, {
postgres: {
sql: 'insert into "person" ("first_name", "last_name", "gender") values ($1, $2, $3), ($4, $5, $6)',
parameters: ['Foo', 'Bar', 'other', 'Baz', 'Spam', 'other'],
},
mysql: {
sql: 'insert into `person` (`first_name`, `last_name`, `gender`) values (?, ?, ?), (?, ?, ?)',
parameters: ['Foo', 'Bar', 'other', 'Baz', 'Spam', 'other'],
},
sqlite: {
sql: 'insert into "person" ("first_name", "last_name", "gender") values (?, ?, ?), (?, ?, ?)',
parameters: ['Foo', 'Bar', 'other', 'Baz', 'Spam', 'other'],
},
})

const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(InsertResult)
expect(result.numInsertedOrUpdatedRows).to.equal(2n)

if (dialect === 'postgres') {
expect(result.insertId).to.be.undefined
} else {
expect(result.insertId).to.be.a('bigint')
}

const inserted = await ctx.db
.selectFrom('person')
.selectAll()
.orderBy('id', 'desc')
.limit(2)
.execute()

expect(inserted).to.containSubset([
{ first_name: 'Foo', last_name: 'Bar', gender: 'other' },
{ first_name: 'Baz', last_name: 'Spam', gender: 'other' },
])
})

if (dialect === 'postgres' || dialect === 'sqlite') {
it('should insert multiple rows', async () => {
it('should insert multiple rows while falling back to default values in partial rows', async () => {
const query = ctx.db
.insertInto('person')
.values([
@@ -522,10 +623,15 @@ for (const dialect of BUILT_IN_DIALECTS) {
})

const result = await query.execute()

expect(result).to.have.length(2)
expect(result).to.containSubset([
{ first_name: 'Foo', last_name: null, gender: 'other' },
{ first_name: 'Baz', last_name: 'Spam', gender: 'other' },
])
})

it('should return data using `returning`', async () => {
it('should insert a row and return data using `returning`', async () => {
const result = await ctx.db
.insertInto('person')
.values({
@@ -552,7 +658,7 @@ for (const dialect of BUILT_IN_DIALECTS) {
last_name: 'Barson',
})

it('conditional returning statement should add optional fields', async () => {
it('should insert a row, returning some fields of inserted row and conditionally returning additional fields', async () => {
const condition = true

const query = ctx.db
@@ -566,11 +672,12 @@ for (const dialect of BUILT_IN_DIALECTS) {
.if(condition, (qb) => qb.returning('last_name'))

const result = await query.executeTakeFirstOrThrow()

expect(result.last_name).to.equal('Barson')
})
})

it('should return data using `returningAll`', async () => {
it('should insert a row and return data using `returningAll`', async () => {
const result = await ctx.db
.insertInto('person')
.values({
27 changes: 27 additions & 0 deletions test/node/src/log-once.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { expect } from 'chai'
import { createSandbox, SinonSpy } from 'sinon'
import { logOnce } from '../../..'

describe('logOnce', () => {
let logSpy: SinonSpy
const sandbox = createSandbox()

before(() => {
logSpy = sandbox.spy(console, 'log')
})

it('should log each message once.', () => {
const message = 'Kysely is awesome!'
const message2 = 'Type-safety is everything!'

logOnce(message)
logOnce(message)
logOnce(message2)
logOnce(message2)
logOnce(message)

expect(logSpy.calledTwice).to.be.true
expect(logSpy.getCall(0).args[0]).to.equal(message)
expect(logSpy.getCall(1).args[0]).to.equal(message2)
})
})
41 changes: 34 additions & 7 deletions test/node/src/update.test.ts
Original file line number Diff line number Diff line change
@@ -113,7 +113,7 @@ for (const dialect of BUILT_IN_DIALECTS) {
expect(person.last_name).to.equal('Catto')
})

it('should update update using a raw expression', async () => {
it('should update one row using a raw expression', async () => {
const query = ctx.db
.updateTable('person')
.set({
@@ -136,10 +136,21 @@ for (const dialect of BUILT_IN_DIALECTS) {
},
})

await query.execute()
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(UpdateResult)
expect(result.numUpdatedRows).to.equal(1n)

const jennifer = await ctx.db
.selectFrom('person')
.where('first_name', '=', 'Jennifer')
.select('last_name')
.executeTakeFirstOrThrow()

expect(jennifer.last_name).to.equal('Jennifer')
})

it('undefined values should be ignored', async () => {
it('should update one row while ignoring undefined values', async () => {
const query = ctx.db
.updateTable('person')
.set({ id: undefined, first_name: 'Foo', last_name: 'Barson' })
@@ -160,11 +171,25 @@ for (const dialect of BUILT_IN_DIALECTS) {
},
})

await query.execute()
const result = await query.executeTakeFirst()

expect(result).to.be.instanceOf(UpdateResult)
expect(result.numUpdatedRows).to.equal(1n)

const female = await ctx.db
.selectFrom('person')
.where('gender', '=', 'female')
.select(['first_name', 'last_name'])
.executeTakeFirstOrThrow()

expect(female).to.deep.equal({
first_name: 'Foo',
last_name: 'Barson',
})
})

if (dialect === 'postgres' || dialect === 'sqlite') {
it('should return updated rows when `returning` is used', async () => {
it('should update some rows and return updated rows when `returning` is used', async () => {
const query = ctx.db
.updateTable('person')
.set({ last_name: 'Barson' })
@@ -196,7 +221,7 @@ for (const dialect of BUILT_IN_DIALECTS) {
])
})

it('conditional returning statement should add optional fields', async () => {
it('should update all rows, returning some fields of updated rows, and conditionally returning additional fields', async () => {
const condition = true

const query = ctx.db
@@ -206,10 +231,11 @@ for (const dialect of BUILT_IN_DIALECTS) {
.if(condition, (qb) => qb.returning('last_name'))

const result = await query.executeTakeFirstOrThrow()

expect(result.last_name).to.equal('Barson')
})

it('should join a table when `from` is called', async () => {
it('should update some rows and join a table when `from` is called', async () => {
const query = ctx.db
.updateTable('person')
.from('pet')
@@ -233,6 +259,7 @@ for (const dialect of BUILT_IN_DIALECTS) {
})

const result = await query.execute()

expect(result[0].first_name).to.equal('Doggo')
})
}

0 comments on commit 606b001

Please sign in to comment.