juanluispaz / ts-sql-query

Type-safe SQL query builder like QueryDSL or JOOQ in Java or Linq in .Net for TypeScript with MariaDB, MySql, Oracle, PostgreSql, Sqlite and SqlServer support.
https://ts-sql-query.readthedocs.io/
MIT License
291 stars 19 forks source link

Infinite loop in query builder #97

Closed Smert closed 1 year ago

Smert commented 1 year ago

Hi. My real production query has a problem with an infinite loop in:

https://github.com/juanluispaz/ts-sql-query/blob/f1420d8e460c91fe3828caff48770d21ae4f917e/src/queryBuilders/SelectQueryBuilder.ts#L989-L998

Select query uses only optional joins.

Before the loop requiredTableOrView.size === 4. After requiredTableOrView.size === 5 and registeredCount (4) !== updatedCount (5).

5th requiredTableOrView was added on line 994 because this 5th table is needed for another join from requiredTableOrView. Columns from 5th table are not selected in the query.

Have you any ideas how to fix this problem?

juanluispaz commented 1 year ago

Hi,

Please, can you give me code to reproduce the problem, or at least what is the query and table definition that generate the situation; If you provide enough information to reproduce it I can fix and publish it tonight.

I'll wait for your answer; and I will investigate by myself; but I need more input from you.

juanluispaz commented 1 year ago

Please, include your ts-sq-query DBConnection configuration, I want to see base class, any configuration set in the DBConnection.

Smert commented 1 year ago

Thanks for the contact! Discord requires numeric code for invite. But I managed to shorten the example:

connection.selectFrom(tParent)
  .optionalJoin(tChild).on(
    tChild.parentId.equals(tParent.id)
  )
  .optionalJoin(tChildPreview).on(tChildPreview.childId.equals(tChild.id))
  .select({
    count: tChildPreview.count
  })
  .query();
Smert commented 1 year ago

Simpler example:

connection.selectFrom(tParent)
  .optionalJoin(tChild).on(
    tChild.parentId.equals(tParent.id)
  )
  .select({
    name: tChild.name
  })
  .query();
Smert commented 1 year ago

DBConnection

export class DBConnection extends MySqlConnection<'Connection'> {
  allowEmptyString = true;
  protected compatibilityMode = true
}

export const createMysqlConnection = (mysqlPool: Pool): DBConnection => new DBConnection(
  ErrorLoggingQueryRunner.create(
    new MySqlPoolQueryRunner(mysqlPool, 'mySql')
  )
);

class ErrorLoggingQueryRunner extends InterceptorQueryRunner<unknown> {
  static create(queryRunner: QueryRunner): ErrorLoggingQueryRunner {
    return new ErrorLoggingQueryRunner(queryRunner, serverLogger.to('ErrorLoggingQueryRunner'));
  }

  constructor(queryRunner: QueryRunner, private logger: Logger) {
    super(queryRunner);
  }

  onQuery(): unknown {
    return;
  }

  onQueryResult(): void {}

  onQueryError(queryType: QueryType, query: string, params: any[], error: any): void {
    this.logger.error(error);
  }
}
juanluispaz commented 1 year ago

I will need you to include the table definition and DBConnection definition as well

Smert commented 1 year ago

Tables:

class ParentTable extends Table<DBConnection, 'ParentTable'> {
  id = this.primaryKey('id', 'int');

  constructor() {
    super('parent')
  }
}
const tParent = new ParentTable();

class ChildTable extends Table<DBConnection, 'ChildTable'> {
  id = this.primaryKey('id', 'int');
  parentId = this.column('parentId', 'int');
  name = this.column('name', 'string');

  constructor() {
    super('child')
  }
}
const tChild = new ChildTable();
juanluispaz commented 1 year ago

Thanks for the additional information; it helped a lot to identify and fix the issue; I added tests that validate this case; additionally, I added an additional validation to prevent the infinite loop and get a proper error in case something fails.

Fix released in ts-sql-query 1.51.0 that is already available for your use.

Please, test it and give me any feedback.

Smert commented 1 year ago

It works. Thank you!