Vincit / objection.js

An SQL-friendly ORM for Node.js
https://vincit.github.io/objection.js
MIT License
7.26k stars 639 forks source link

Invalid SQL syntax when loading a HasOneThroughRelation with eager() #1011

Closed petercpwong closed 6 years ago

petercpwong commented 6 years ago

Running a query with eager() on a HasOneThroughRelation that has a table name prefixed with a schema will yield an SQL error:

invalid reference to FROM-clause entry for table

Here's a sample of what the SQL looks like:

SELECT "myschema"."table_c".*, "myschema"."table_b"."table_a_id" AS "objectiontmpjoin0"
FROM "myschema"."table_c" 
INNER JOIN "myschema"."table_b" AS "myschema.table_b" 
ON "myschema"."table_c"."id" = "myschema"."table_b"."table_c_id" 
WHERE "myschema"."table_b"."table_a_id" IN (?)

Notice the problem is in the alias of the join table:

INNER JOIN "myschema"."table_b" AS "myschema.table_b"

The table alias "myschema.table_b" should be dropped since it's referenced as myschema"."table_b" everywhere else in the statement.

Steps to reproduce:

koskimas commented 6 years ago

Providing schema like that isn't supported by knex and therefore isn't supported by objection. Objection still has a limited support for the things that do work with knex, but as long as knex doesn't support giving schema as a part of the reference, objection cannot really guarantee support or implement most of the things. If you do figure out a simple and clean way to support this use case, I can consider merging a PR.

koskimas commented 6 years ago

Knex does support schemas through the withSchema method. I haven't tried this, but maybe you could do something like this:

Create a shared base class

class SchemaModel extends Model {
  // Override the static `query` method to add a schema to each query
  static query(...args) {
    const query = super.query(...args)

    if (this.schemaName) {
      query.withSchema(this.schemaName)
    }

     return query
  }
}

Inherit all your models from that class

class SomeModel exends SchemaModel {
  static tableName() {
    return 'table_a'
  }

  static schemaName() {
    return 'myschema'
  }
}
heisian commented 6 years ago

@petercpwong see related multi-schema issues: 1.2.0 changes ManyToMany/HasOneThrough join SQL Multi-schema Relationships [PostgreSQL] add multi-schema integration tests for Postgres

As a note, I tried using schemaName as koskimas suggested, but that did not work for me in cross-schema JOINs. So I currently use tableName() { return 'mySchema.tableName' }.

Current workaround is to version-lock to 1.1.10 while a fix is thought of...

koskimas commented 6 years ago

Actually this is a duplicate of #1007