kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
10.71k stars 271 forks source link

MySQL Introspector fails because of incorrect casing #135

Closed Devvypaws closed 2 years ago

Devvypaws commented 2 years ago

Overview

The MySQL Introspector selects columns from information_schema.columns in lowercase, but parseTableMetadata() expects the columns to be uppercase, causing it to throw.

Environment

Node: 18.6.0 Kysely: 0.19.12 and 0.21.1 MySQL: 5.7.33

Research

The result of the query MySQL Introspector uses was inspected and found to output the following:

{
  column_name: 'id',
  column_default: null,
  table_name: 'addresses',
  table_schema: '[REDACTED]',
  is_nullable: 'NO',
  data_type: 'int',
  extra: 'auto_increment'
}

However, parseTableMetadata() expects these fields to be uppercase:

#parseTableMetadata(columns) {
// ...
        let table = tables.find((tbl) => tbl.name === it.TABLE_NAME);
// ...
                name: it.TABLE_NAME,
                schema: it.TABLE_SCHEMA,
// ...
            name: it.COLUMN_NAME,
            dataType: it.DATA_TYPE,
            isNullable: it.IS_NULLABLE === 'YES',
            isAutoIncrementing: it.EXTRA.toLowerCase().includes('auto_increment'),
            hasDefaultValue: it.COLUMN_DEFAULT !== null,
// ...
}

This causes it to throw the following error:

[REDACTED]/node_modules/kysely/dist/cjs/dialect/mysql/mysql-introspector.js:63
                isAutoIncrementing: it.EXTRA.toLowerCase().includes('auto_increment'),
                                             ^

TypeError: Cannot read properties of undefined (reading 'toLowerCase')
    at [REDACTED]/node_modules/kysely/dist/cjs/dialect/mysql/mysql-introspector.js:63:46

It appears the casing of the SELECT statement affects the returned results, as confirmed by both a manual query of the database and also modifying

let query = this.#db
  .selectFrom('information_schema.columns')
  .select([
      'column_name',
      'column_default',
      'table_name',
      'table_schema',
      'is_nullable',
      'data_type',
      'extra',
  ])

to use uppercase, which returns the expected column names and succeeds.

koskimas commented 2 years ago

Kysely has tests for this and they all pass.

koskimas commented 2 years ago

Well, your fix seems to also work so I'll merge it.

koskimas commented 2 years ago

Fixed in 0.21.2. Thank you!

Devvypaws commented 2 years ago

Yeah, I was confused as to why it was happening because I expected the returned columns to just match the case they are in the system. But even testing it out myself, I got that same behavior. Nonetheless, the casings matching the expected output couldn't hurt. :-)