Add 'sqlite3' to the RETURNING_CLIENTS constant in KnexAdapter

[renancpin]

Summary:

Add 'sqlite3' to the @feathersjs/knex/src/adapter.ts:27's RETURNING_CLIENTS const list, in order to return the correct id column from INSERT statements

I'm a new user of feathers, and recently created my first project, and chose sqlite as the database since I was just toying around

I tried to change the primary key on the user and other generated service's tables to use a uuid instead of the incremental integer, but no matter what I tried to change in the migrations and schemas, the create route was always throwing a 404 NotFound, even if the row was correctly inserted:

{
    "name": "NotFound",
    "message": "No record found for id '1'",
    "code": 404,
    "className": "not-found"
}

So my migration's up() was pretty much:

export async function up(knex: Knex): Promise<void> {
  await knex.schema.createTable('users', (table) => {
    // changed from .increments() to .uuid()    
    table.uuid('id').primary().defaultTo(knex.fn.uuid())

    // changed usernameField from "email" to "username" at the correct places in local auth strategy and schemas
    table.string('username').unique()
    table.string('password')
  })
}

So I decided to look into the _create(data, params) method at the KnexAdapter at @feathersjs/knex , and I realized the issue was probably related to this:

At @feathersjs/knex/src/adapter.ts, lines 243 to 256:

// This probably tells knex to add a "RETURNING <primaryKeyColumn>"
// clause (available at only a few db clients) at the end of the INSERT statement
const returning = RETURNING_CLIENTS.includes(client as string) ? [this.id] : []
const rows: any = await this.db(params)
  .insert(data, returning, { includeTriggerModifications: true })
  .catch(errorHandler)

// This one seems to try and pick the primary key from results...
const id = data[this.id] || rows[0][this.id] || rows[0]

if (!id) {
  return rows as Result[]
}

// ...to issue a SELECT statement and return the complete row
return this._get(id, {
  ...params,
  query: _.pick(params?.query || {}, '$select')
})

And at line 27:

const RETURNING_CLIENTS = ['postgresql', 'pg', 'oracledb', 'mssql']

So, the sqlite or sqlite3 is missing from this list.

Then I looked into SQLite's official documentation and found out that:

1. SQLite adds a ROWID column containing an uniquely indexed INTEGER for all tables, by default
2. the RETURNING clause is already implemented on SQLite, in the likes of PostgreSQL

It seems that sqlite3 returns this ROWID like [1] by default when inserting, which is then used as the primary key for the subsequent _get() call, probably causing the NotFound error I've mentioned.

Then I tried altering this line at adapter.js to include sqlite3 on the RETURNING_CLIENTS list:

const RETURNING_CLIENTS = ['postgresql', 'pg', 'oracledb', 'mssql', 'sqlite3']

and voilà, got the expected response from calling the create method on the API:

{
    "id": "124bfd37-xxxx-xxxx-xxxx-b78x0xx06cd6",
    "username": "some_random@email.com"
}

Therefore, I'd like to suggest simply including sqlite to the list of clients that implement the RETURNING clause, to correctly return created items at insert calls

[daffl]

Thank you for digging into this. I believe your suggested fix should work. Just for reference, here are some issues I think are related:

• custom ids are returned empy array on create #3049
• fix(knex): Ensure custom ids are returned on create #2934
• .returning() is not supported by mysql and will not have any effect. #2989