Skip to content
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

Sqlite multi-insert fails to convert Object to JSON string but does for single insert #5769

Open
code-ape opened this issue Dec 13, 2023 · 0 comments

Comments

@code-ape
Copy link
Collaborator

code-ape commented Dec 13, 2023

Environment

Knex version: 3.0.1
Database + version: sqlite3 version 5.1.6
OS: MacOS 14.1
Node: 20.3.1

Bug

  1. Explain what kind of behavior you are getting and how you think it should work.

When using the SQLite driver with a JSON column, the behavior between inserting a single record and multiple records is different.

await knex.from('my_table').insert([
   { my_column: { a: 1 }},
])
// Results in conversion being done via JSON.stringify:
//   sql: 'insert into `my_table` (`my_column`) values (?)',
//   bindings: [ '{"a":1}' ]

await knex.from('my_table').insert([
   { my_column: { b: 2 }},
   { my_column: { c: 3 }},
])
// Results in conversion NOT being done:
//   sql: 'insert into `my_table` (`my_column`) select ? as `my_column` union all select ? as `my_column`',
//   bindings: [ { b: 2 }, { c: 3 } ]
// This causes the string `[object Object]` to be inserted as `.toString()` is called by the SQLite3 driver.
  1. Error message

None, this occurs silently.

  1. Reduced test code, for example in https://npm.runkit.com/knex or if it needs real database connection to MySQL or PostgreSQL, then single file example which initializes needed data and demonstrates the problem.
import Knex from 'knex'

async function main() {
  console.log('Creating in-memory database ...')

  const knex = Knex({
    client: 'sqlite3',
    connection: ':memory:',
    useNullAsDefault: true,
  })

  console.log('Creating table `my_table` ...')
  await knex.schema.createTable('my_table', (table) => {
    table.json('my_column').notNullable()
  })

  console.log('Doing single insert ...')
  const singleInsert = knex.from('my_table').insert([
    { my_column: { a: 1 }},
  ])
  console.log('Single insert SQL:', singleInsert.toSQL().toNative())
  await singleInsert

  console.log('Doing multi insert ...')
  const multiInsert = knex.from('my_table').insert([
    { my_column: { b: 2 }},
    { my_column: { c: 3 }},
  ])
  console.log('Multi insert SQL:', multiInsert.toSQL().toNative())
  await multiInsert

  console.log('Getting all rows from table...')
  const rows = await knex.from('my_table').select('*')
  console.log(rows)

  await knex.destroy()
}

main()

// Output:
//   Creating in-memory database ...
//   Creating table `my_table` ...
//   Doing single insert ...
//   Single insert SQL: {
//     sql: 'insert into `my_table` (`my_column`) values (?)',
//     bindings: [ '{"a":1}' ]
//   }
//   Doing multi insert ...
//   Multi insert SQL: {
//     sql: 'insert into `my_table` (`my_column`) select ? as `my_column` union all select ? as `my_column`',
//     bindings: [ { b: 2 }, { c: 3 } ]
//   }
//   Getting all rows from table...
//   [
//     { my_column: '{"a":1}' },
//     { my_column: '[object Object]' },
//     { my_column: '[object Object]' }
//   ]
  1. Additional Notes

Also, an initial quick dive into the codebase seems to suggest the following code is the cause of this. The difference between inserting a single item and multiple is this.client.parameterize() for the single and this.client.parameter() for multiple.

The Sqlite driver also appears to NOT correctly to JSON.stringify() as shown above for values passed to .update() as well. This also seems to be from _prepUpdate() using this.client.parameter() instead of this.client.parameterize().

if (insertData.values.length === 1) {
const parameters = this.client.parameterize(
insertData.values[0],
this.client.valueForUndefined,
this.builder,
this.bindingsHolder
);
sql += ` values (${parameters})`;
const { onConflict, ignore, merge } = this.single;
if (onConflict && ignore) sql += this._ignore(onConflict);
else if (onConflict && merge) {
sql += this._merge(merge.updates, onConflict, insertValues);
const wheres = this.where();
if (wheres) sql += ` ${wheres}`;
}
const { returning } = this.single;
if (returning) {
sql += this._returning(returning);
}
return {
sql,
returning,
};
}
const blocks = [];
let i = -1;
while (++i < insertData.values.length) {
let i2 = -1;
const block = (blocks[i] = []);
let current = insertData.values[i];
current = current === undefined ? this.client.valueForUndefined : current;
while (++i2 < insertData.columns.length) {
block.push(
this.client.alias(
this.client.parameter(
current[i2],
this.builder,
this.bindingsHolder
),
this.formatter.wrap(insertData.columns[i2])
)
);
}
blocks[i] = block.join(', ');
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant