sql/objection where clause does not work with relations: 'column reference "id" is ambiguous'

[MoJo2600]

I think I found another bug with relations in 'ucast'.

const query = AwsAccountModel.query();

const { can, build } = new AbilityBuilder(Ability);
can('read', 'AwsAccountModel', { 'id': { $in: ['0001', '0002'] } });
const ability = build();

const ast = rulesToAST(ability, 'read', 'AwsAccountModel');
const newQuery = interpret(
    ast,
    query
);
newQuery.debug().then()

returns a working sql query: select "aws_accounts".* from "aws_accounts" where "id" in($1, $2)

If i add a new rule to check for a related property, the sql query is not working anymore. It does not matter if the query for 'id' is 'in' or 'eq'.

const query = AwsAccountModel.query();

const { can, build } = new AbilityBuilder(Ability);
can('read', 'AwsAccountModel', { 'technicalContact.id': 'xxx' });
can('read', 'AwsAccountModel', { 'id': { $in: ['0001', '0002'] } });
const ability = build();

const ast = rulesToAST(ability, 'read', 'AwsAccountModel');
const newQuery = interpret(
    ast,
    query
);
newQuery.debug().then()

This will return the sql query:

select
   "aws_accounts".* 
from
   "aws_accounts" 
   inner join
      "users" as "technicalContact" 
      on "technicalContact"."id" = CAST("aws_accounts"."data" #>> '{TechnicalContact}' AS text) 
where
   (
      "id" in
      (
         $1,
         $2
      )
      or "technicalContact"."id" = $3
   )

Which generates the error column reference "id" is ambiguous.

The field "id" in the where clause should be "aws_accounts"."id".

I can't set the rule to be something like can('read', 'AwsAccountModel', { 'aws_accounts.id': { $in: ['0001', '0002'] } });, because the dot notation is always interpreted as a relation. Relation fields are always added in the correct notation "relationName"."field".

I think the root table in the where clause should always be prefixed with the table name.

[MoJo2600]

I created a unittest which shows this behaviour:

  it('does prefix where clause when relation exists', () => {
    const condition = new CompoundCondition('or', [
      new Field('eq', 'id', 1),
      new Field('eq', 'projects.name', 'test'),
    ])
    const query = interpret(condition, User.query())

    expect(query.toKnexQuery().toString()).to.equal(linearize`
      select "users".*
      from "users"
        inner join "projects" on "projects"."user_id" = "users"."id"
      where ("user"."id" = 1 or "projects"."name" = 'test'
    `.trim())
  })

fails currently with:

      -select "users".* from "users" inner join "projects" on "projects"."user_id" = "users"."id" where ("id" = 1 or "projects"."name" = 'test')
      +select "users".* from "users" inner join "projects" on "projects"."user_id" = "users"."id" where ("user"."id" = 1 or "projects"."name" = 'test'

[stalniy]

By the way, I did some work in alpha branch which is not merged in master. Very likely it’s fixed. I use ucast/sql for internal ORM library for one of my customers

[stalniy]

But I’ve not updated integration with other orm

[MoJo2600]

Cool, i will try the alpha branch!

[MoJo2600]

Ok, just to let you know. I did some testing in alpha branch. It does work, when I add the rootAlias property to the options object.

interpreter.spec.ts:

    it('prefixes primary table properties on join', () => {
      const optionsNew: SqlQueryOptions = {
        ...pg,
        joinRelation: () => true,
        rootAlias: 'user',  // <- does not work without this
      }

      const condition = new CompoundCondition('and', [
        new Field('eq', 'id', 5),
        new Field('eq', 'project.name', 'test'),
      ])
      const [sql] = interpret(condition, optionsNew)
      expect(sql).to.equal('("user"."id" = $1 and "project"."name" = $2)')
    })

Currently there is no way for objection to set this options. I'm just guessing how it is supposed to work, but i think it should be set somehow automatically from the Query object? I'm currently still trying to understand how the code is working, so excuse me if I'm bothering you :)

[cml391]

Hi, I'm also having this issue when trying to use interpret that is returned from the objection piece of the package (rather than the default SQL one) since as you mentioned you haven't updated the ORM pieces yet. If someone hasn't started working on it, I would be happy to help write a small fix to all people to pass the SqlQueryOptions through here. Let me know! Thanks

[stalniy]

@cml391 would be awesome! Sorry for not being active on this libs for now. Just busy with other stuff which we need for our production systems

[cml391]

Great @stalniy, I'll make a fork and PR for that at some point tomorrow