refactor: Migrate FindOperator SQL generation into QueryBuilder

[imnotjames]

Migrate the code from FindOperator.toSql into QueryBuilder.computeFindOperatorExpression.

The QueryBuilder is the de-facto location for all of the SQL Dialect generation.
Let's move the FindOperator.toSQL to live there too.

Effectively, we were passing a bunch of context from within the QueryBuilder back into the
FindOperator meaning we couldn't actually use FindOperator without QueryBuilder.
The FindOperator had most of its code in a mechanism that wasn't even used by all drivers
(eg, Mongo).

QueryBuilder did not have encapsulation of the SQL dialect & more in-depth changes
to the way we generate SQL based on the FindOperator can't be as easily done as
we didn't have context of the rest of the QueryBuilder.

[pleerock]

Looks good, thank you

[BasileTrujillo]

Hi there,
I just updated typeOrm to the last release and this changes broked my projet.
Actually my projet is a tool that auto build complex typeOrm request using FindOperatior QueryBuilder and the .toSQL().

The move to QueryBuilder make sense but the encapsulation has changed from public (in FindOperator) to protected (in QueryBuilder) locking the function to be no longer usable outside the QueryBuilder itself...

Could you consider declaring computeFindOperatorExpression as public ?

[imnotjames]

Hey there - while the toSQL API was public, it was not documented and was only really public seemingly so query builder could use it.

Could you give examples of how you used this?

I'd also suggest opening a new issue regarding that.

[stelescuraul]

@imnotjames Before opening an issue, I would like your input on this. This also broke my project where I have custom operators created:
EG: I have an ilike operator that was extending the FindOperator and modifing the toSql. I understand the reason for the refactor, but is there a standard way of creating custom operators now ?
Let me know if you need more info and i can open an issue on this.

[imnotjames]

@stelescuraul I think that use case is closer to the exception than the norm - but a path forward would be good - totally get you there.

We do already have an escape hatch in Raw -- would wrapping Raw work for some of that? Though you'd lose out on some parameters.

Might be worthwhile to extend raw handling to allow for that use case. EG, passing parameters to the Raw callback if it has em.. that way you could implement your custom operator as..

export const ILike = (value) => Raw("raw", (alias, param) => `${alias} ILIKE ${param}`, value);

Or something like that?

Either way, definitely open the issue. Would be good to continue the discussion there.

[cjnoname]

same thing here, my ILike doesn't work. Could you fix that ASAP?

[pleerock]

we already have an ILike out of the box, you probably don't need a custom one anymore?

[BasileTrujillo]

Hey there - while the toSQL API was public, it was not documented and was only really public seemingly so query builder could use it.

Could you give examples of how you used this?

I'd also suggest opening a new issue regarding that.

Actually the part of my projet that use toSQL is a function witch automatically add conditions and joins to the where function of a QueryBuilder (or SelectQueryBuilder in my case).

Here is a piece of code that use the toSQL() function in my projet:

  private addFiltersToQueryBuilder(queryBuilder: SelectQueryBuilder<any>, whereValue: FindOperator<any>) {

    // ...

    const whereCondition = [];
    const whereConditionParams = {};

    whereCondition.push(whereValue.toSql(this.connection, `${join.alias}.${whereKey}`, [`:${paramName}`]));
    whereConditionParams[paramName] = whereValue.value;

    const condition = whereCondition.join(' AND ');
    queryBuilder.where(condition, whereConditionParams);
  }

That's was the only way I found in the TypeORM lib to correctly build conditions over predefined joins.

Formerly I simply map an input object of filters coming from a GraphQL query into a QueryBuilder request.

tableA (filters: {
    name: {contains: "test"}
    tableB: {
      enabled: true
    }
  }) {
  hits {
  // ...
  }
}

Will create a QueryBuilder with where conditions and joins recursively. This way I can work with a kind of badass repository find() method :)

[imnotjames]

That wasn't a supported API & the way you were using it is why I made it private - because it's likely going to change and break again in the future. Sorry that it stopped working for you. ):

Off the top of my head, if you want to write something that builds your own queries in a way that we don't support, you can maintain a version of the translation separately, or use a more specialized library like knex, or you can always open a new issue for feature requests.

[mo4islona]

@pleerock

we already have an ILike out of the box, you probably don't need a custom one anymore?

Sorry, but i dont see ILike operator in 0.2.28 version. What do you mean "out of box"?

So you made breaking change in a minor release. Do you have a "safe" branch/channel/etc without those?

[imnotjames]

ILIKE is in next - I'll look at backporting that into master soon. My mistake, I had thought it was in the main branch as well.

So you made breaking change in a minor release. Do you have a "safe" branch/channel/etc without those?

In a patch release, not a minor release.

• The toSql method was not documented as part of a public API and was subject to change.
• TypeORM is pre-1.0 and while best effort is exerted to prevent breaking changes they may happen.

[mo4islona]

@imnotjames thanks!

[imnotjames]

Either way, I'm sorry that this change inadvertently caused issues with your code.

We actually just merged in a change for Raw to support parameters. Perhaps that will help your use case?

Raw((columnAlias) => ${columnAlias} ILIKE :value, { value: "foo%" })

For the backport to bring ILIKE into 0.2.x, #6862

[sanderevers]

This toSql change also broke our project. We were (ab)using it to insert some custom SQL and still make use of the parameter mechanism. Luckily we can now fix it with #6850