Ambiguous column when joining table using createQuery and custom hook together

[alwex]

Steps to reproduce

Create a new project, register a service, create a global hook and override createQuery to add join columns.

export class ChatMessagesService extends KnexService<
  ChatMessagesResult,
  ChatMessagesData,
  ChatMessagesParams
> {
  createQuery(params: ChatMessagesParams): any {
    const query = super.createQuery(params)

    query
      .join('users', 'chat_messages.userId', 'users.id')
      .join('user_schools', 'users.id', 'user_schools.userId')
      .select('user_schools.schoolId as schoolId')

    return query
  }
}

implement a softDelete hook as follow:

export const softDeleteRead = async (context: HookContext) => {
  const { method, params, path, type } = context
  const { query = {} } = params

  if (type !== 'before') {
    return context
  }

  if (path === 'authentication') {
    return context
  }

  if (method === 'remove') {
    return context
  }

  context.params.query = {
    ...query,
    deletedAt: null,
  }

  return context
}

register the softDelete hook at the app level

// Register hooks that run on all service methods
app.hooks({
  around: {
    all: [logErrorHook],
  },
  before: {
    all: [softDeleteRead], // <====== HERE
    create: [createdAt, deletedAt],
    update: [notAllowed],
    patch: [updatedAt],
  },
  after: {},
  error: {},
})
// Register application setup and teardown hooks here
app.hooks({
  setup: [],
  teardown: [],
})

Expected behavior

when using find I am expecting to get the message results along with the schoolId populated from the join query

Actual behavior

an error is thrown as the column deletedAt is not prefixed with the table name and the column become ambiguous as the deletedAt column is present on all tables

System configuration

using 5.0.0-pre.29 as I was not able to easily upgrade to 5.0.x, but after reading the actual knex adapter, it seems that the issue is also present on latest 5.0.5

I have fixed the issue locally with a patch:

diff --git a/node_modules/@feathersjs/knex/lib/adapter.js b/node_modules/@feathersjs/knex/lib/adapter.js
index 4765ae0..4283485 100644
--- a/node_modules/@feathersjs/knex/lib/adapter.js
+++ b/node_modules/@feathersjs/knex/lib/adapter.js
@@ -58,13 +58,16 @@ class KnexAdapter extends adapter_commons_1.AdapterBase {
         return schema ? Model.withSchema(schema).table(table) : Model(table);
     }
     knexify(knexQuery, query = {}, parentKey) {
+        const { table, id } = this;
         const knexify = this.knexify.bind(this);
         return Object.keys(query || {}).reduce((currentQuery, key) => {
             const value = query[key];
             if (commons_1._.isObject(value)) {
                 return knexify(currentQuery, value, key);
             }
-            const column = parentKey || key;
+            const columnRaw = parentKey || key
+            const column = columnRaw.includes('.') ? columnRaw : `${table}.${columnRaw}`
+
             const method = METHODS[key];
             if (method) {
                 if (key === '$or' || key === '$and') {
@@ -92,8 +95,9 @@ class KnexAdapter extends adapter_commons_1.AdapterBase {
         const builder = this.db(params);
         // $select uses a specific find syntax, so it has to come first.
         if (filters.$select) {
+            const select = filters.$select.map((column) => (column.includes('.') ? column : `${table}.${column}`))
             // always select the id field, but make sure we only select it once
-            builder.select(...new Set([...filters.$select, `${table}.${id}`]));
+            builder.select(...new Set([...select, `${table}.${id}`]));
         }
         else {
             builder.select(`${table}.*`);
diff --git a/node_modules/@feathersjs/knex/src/adapter.ts b/node_modules/@feathersjs/knex/src/adapter.ts
index a3f3495..cce6972 100644
--- a/node_modules/@feathersjs/knex/src/adapter.ts
+++ b/node_modules/@feathersjs/knex/src/adapter.ts
@@ -77,6 +77,7 @@ export class KnexAdapter<
   }

[alwex]

Here is an update of the patch file that cover the patch method as well

diff --git a/node_modules/@feathersjs/knex/lib/adapter.js b/node_modules/@feathersjs/knex/lib/adapter.js
index 4765ae0..1a2b013 100644
--- a/node_modules/@feathersjs/knex/lib/adapter.js
+++ b/node_modules/@feathersjs/knex/lib/adapter.js
@@ -58,13 +58,16 @@ class KnexAdapter extends adapter_commons_1.AdapterBase {
         return schema ? Model.withSchema(schema).table(table) : Model(table);
     }
     knexify(knexQuery, query = {}, parentKey) {
+        const { table, id } = this;
         const knexify = this.knexify.bind(this);
         return Object.keys(query || {}).reduce((currentQuery, key) => {
             const value = query[key];
             if (commons_1._.isObject(value)) {
                 return knexify(currentQuery, value, key);
             }
-            const column = parentKey || key;
+            const columnRaw = parentKey || key
+            const column = columnRaw.includes('.') ? columnRaw : `${table}.${columnRaw}`
+
             const method = METHODS[key];
             if (method) {
                 if (key === '$or' || key === '$and') {
@@ -92,8 +95,9 @@ class KnexAdapter extends adapter_commons_1.AdapterBase {
         const builder = this.db(params);
         // $select uses a specific find syntax, so it has to come first.
         if (filters.$select) {
+            const select = filters.$select.map((column) => (column.includes('.') ? column : `${table}.${column}`))
             // always select the id field, but make sure we only select it once
-            builder.select(...new Set([...filters.$select, `${table}.${id}`]));
+            builder.select(...new Set([...select, `${table}.${id}`]));
         }
         else {
             builder.select(`${table}.*`);
@@ -192,8 +196,18 @@ class KnexAdapter extends adapter_commons_1.AdapterBase {
                 ...(((_a = params === null || params === void 0 ? void 0 : params.query) === null || _a === void 0 ? void 0 : _a.$select) ? { $select: (_b = params === null || params === void 0 ? void 0 : params.query) === null || _b === void 0 ? void 0 : _b.$select } : {})
             }
         };
+
+        const prefixedData = Object.entries(data).reduce((result, [column, value]) => {
+            if (column.includes('.')) { 
+            result[column] = value;
+            } else {
+            result[`${this.table}.${column}`] = value;
+            }
+            return result;
+        }, {})
+
         const builder = this.createQuery(updateParams);
-        await builder.update(data);
+        await builder.update(prefixedData);
         const items = await this._findOrGet(null, updateParams);
         if (id !== null) {
             if (items.length === 1) {

Has it been fixed on latest version of the adapter?