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

Metadata added to queries since 0.15.0 #3514

Closed
immortalx opened this issue Nov 5, 2019 · 8 comments
Closed

Metadata added to queries since 0.15.0 #3514

immortalx opened this issue Nov 5, 2019 · 8 comments

Comments

@immortalx
Copy link

Environment

Knex version: 0.20.1 mysql2 driver
Database + version: mysql 5.7
OS: Ubuntu 16

Bug

The same query result comes with the metadata TextRow on Knex 0.20.1, this is true since 0.15.0.
Currently I'm using version 0.14.6 and tried to upgrade to the latest version.

export default knex({
  client: 'mysql2',
  connection: {
    host: process.env.DB_HOST,
    user: process.env.DB_USER,
    password: process.env.DB_PASSWORD,
    database: process.env.DB_NAME,
    charset: process.env.DB_CHARSET
  },
  debug: false,
  pool: {
    min: Number(process.env.DB_POOL_MIN),
    max: Number(process.env.DB_POOL_MAX)
  }
})

Query:

 const d = await knex(table).whereIn('uid', ids).select('id')

0.14.6 Results:

[{
     id: 1
}]

0.20.1 Results:

[ TextRow {
     id: 1
}]
@kibertoad
Copy link
Collaborator

Are you sure you are using same version of mysql driver in both cases?

@immortalx
Copy link
Author

Yes, even tried different versions of mysql2 to make sure.

@elhigu
Copy link
Member

elhigu commented Nov 5, 2019

That sound as expected. Why is that a bug? Automatic conversion from driver specific result objects to plain JS objects was removed around 0.15, because it was accountable for over 50% of row deserialization time. If you like to get the old behaviour back you can add your own postProcessResponse handler to do the conversion.

@elhigu elhigu closed this as completed Nov 5, 2019
@immortalx
Copy link
Author

@elhigu Sorry, i missed that changelog line in 0.15:

Testing removal of 'skim' #2520, Now rows are not converted to plain js objects, returned row objects might have changed type with oracle, mssql, mysql and sqlite3

Thanks for your explanation!

@jmrossy
Copy link

jmrossy commented Jun 19, 2020

@elhigu Can you elaborate on why automatic conversion from driver specific result objects was removed? Is without that, it makes it much less convenient to share code across different drivers.

The intro paragraph in the knex docs include this bit: standardized responses between different query clients and dialects. which sets expectations that knex will normalize responses.

@elhigu
Copy link
Member

elhigu commented Jun 22, 2020

@jmrossy contents and accessing the data was the same for all the drivers. So data can be accessed the same way like from plain JS object.

Conversion was a real performance bottleneck for queries containing thousands of results.

standardized responses between different query clients and dialects

Knex still standardizes the results, because it still fetch or build if necessary the internal array of result row objects from the database.

So with all databases you can do this:

   const allRows = await knex('table');
   allRows.forEach(row => {
     console.log('Name:', row.name);
   });

@jmrossy
Copy link

jmrossy commented Jun 22, 2020

@elhigu Thanks for clarifying.
I found this works as expected for most queries but I did an inconsistency btwn Postgres and MySql for a count query. I worked around it by using Object.values() instead of accessing the count property directly.

  const result = await knex(TABLE)
    .count(COLUMNS.address)
    .first()

  const count = Object.values(result)[0]

@elhigu
Copy link
Member

elhigu commented Jun 22, 2020

Count query result different is another story... it is not different because of that change. Would be good to have some way to get those in a crossdb compatible manner.

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

4 participants