Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend dirty_read and possibly hints in mssql joins #4764

Closed
alfonsoal1983 opened this issue Sep 17, 2019 · 3 comments · Fixed by #8507
Closed

Extend dirty_read and possibly hints in mssql joins #4764

alfonsoal1983 opened this issue Sep 17, 2019 · 3 comments · Fixed by #8507

Comments

@alfonsoal1983
Copy link

alfonsoal1983 commented Sep 17, 2019

Issue type:

[x] question
[ ] bug report
[x] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[x] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[x] latest
[ ] @next
[ ] 0.x.x (or put your version here)

Hello, when making a simple query dirty_read works fine:

getRepository(Car).createQueryBuilder('cars').setLock('dirty_read').getOne()

This would result in:

SELECT cars.allFields FROM CAR cars WITH(NOLOCK)

The problem comes when I have to extend dirty_read to a query with joins. Example:

getRepository(Car).createQueryBuilder('cars').setLock('dirty_read')
.innerJoin('car.tires', 'tires')
.getOne()

This would result in:

SELECT cars.allFields, tires.allFields FROM CAR car WITH(NOLOCK) INNER JOIN TIRE tires ON tires.ID = cars.TIREID

The dirty_read will only apply to the main from so that if there is any open transaction that affects the join table, the query will be waiting for the transaction, the idea is to put the dirty_read in the join.

Is there any way to make dirty_read extend to joins?

If not. Can I implement a function that meets that functionality? I have tried it locally and it works quite well, in the same way, you could use that function to apply more mssql hints (https://docs.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-table?view=sql-server-2017), something like this (it's just an incomplete example, the final code would be more readable):

SelectQueryBuilder.ts

innerJoinHints(entityOrProperty: Function|string|((qb: SelectQueryBuilder<any>) => SelectQueryBuilder<any>), alias: string, condition: string = "", parameters?: ObjectLiteral, hints?: string[]): this {
        this.join("INNER", entityOrProperty, alias, condition, parameters, undefined, undefined, hints);
        return this;
    }

const hints = joinAttr.hints ? ` WITH (${joinAttr.hints})` : '';
return " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + hints + " ON " + this.replacePropertyNames(condition + appendedCondition);

JoinAttribute.ts

hints?: string[];

And call it with:

.innerJoinHints('car.tires', 'tires', null, null, ['NOLOCK'])

the only drawback is that it would only apply to mssql which is the one that uses this nomenclature.

@icecreamparlor
Copy link
Contributor

icecreamparlor commented Jan 5, 2022

I'm facing similar issues.
How about adding WITH(NOLOCK) option if expressionMap.lockMode is "dirty_read" ?
I didn't tried it locally, but rough idea about this suggestion is below.

// SelectQueryBuilder.ts
protected createJoinExpression() {
      /**************************** Check if I should add WITH (NOLOCK) Option on Join ****************************/
      const isDirtyRead =
        this.connection.driver instanceof SqlServerDriver &&
        this.expressionMap.lockMode === "dirty_read";
      const withNolock = " WITH (NOLOCK)";
      /**************************** Check if I should add WITH (NOLOCK) Option on Join ****************************/

      const joins = this.expressionMap.joinAttributes.map((joinAttr) => {
        const relation = joinAttr.relation;
        const destinationTableName = joinAttr.tablePath;
        const destinationTableAlias = joinAttr.alias.name;
        let appendedCondition = joinAttr.condition
          ? " AND (" + joinAttr.condition + ")"
          : "";
        const parentAlias = joinAttr.parentAlias;

        // if join was build without relation (e.g. without "post.category") then it means that we have direct
        // table to join, without junction table involved. This means we simply join direct table.
        if (!parentAlias || !relation) {
          const destinationJoin = joinAttr.alias.subQuery
            ? joinAttr.alias.subQuery
            : this.getTableName(destinationTableName);
          return (
            " " +
            joinAttr.direction +
            " JOIN " +
            destinationJoin +
            " " +
            this.escape(destinationTableAlias) +

             /**************************** Added This Line ****************************/
            (isDirtyRead ? withNolock : "") +
             /**************************** Added This Line ****************************/

            (joinAttr.condition
              ? " ON " + this.replacePropertyNames(joinAttr.condition)
              : "")
          );
        }

        // if real entity relation is involved
        if (relation.isManyToOne || relation.isOneToOneOwner) {
          // JOIN `category` `category` ON `category`.`id` = `post`.`categoryId`
          const condition = relation.joinColumns
            .map((joinColumn) => {
              return (
                destinationTableAlias +
                "." +
                joinColumn.referencedColumn!.propertyPath +
                "=" +
                parentAlias +
                "." +
                relation.propertyPath +
                "." +
                joinColumn.referencedColumn!.propertyPath
              );
            })
            .join(" AND ");

          return (
            " " +
            joinAttr.direction +
            " JOIN " +
            this.getTableName(destinationTableName) +
            " " +
            this.escape(destinationTableAlias) +

             /**************************** Added This Line ****************************/
            (isDirtyRead ? withNolock : "") +
             /**************************** Added This Line ****************************/

            " ON " +
            this.replacePropertyNames(condition + appendedCondition)
          );
        } else if (relation.isOneToMany || relation.isOneToOneNotOwner) {
          // JOIN `post` `post` ON `post`.`categoryId` = `category`.`id`
          const condition = relation
            .inverseRelation!.joinColumns.map((joinColumn) => {
              if (
                relation.inverseEntityMetadata.tableType === "entity-child" &&
                relation.inverseEntityMetadata.discriminatorColumn
              ) {
                appendedCondition +=
                  " AND " +
                  destinationTableAlias +
                  "." +
                  relation.inverseEntityMetadata.discriminatorColumn
                    .databaseName +
                  "='" +
                  relation.inverseEntityMetadata.discriminatorValue +
                  "'";
              }

              return (
                destinationTableAlias +
                "." +
                relation.inverseRelation!.propertyPath +
                "." +
                joinColumn.referencedColumn!.propertyPath +
                "=" +
                parentAlias +
                "." +
                joinColumn.referencedColumn!.propertyPath
              );
            })
            .join(" AND ");

          return (
            " " +
            joinAttr.direction +
            " JOIN " +
            this.getTableName(destinationTableName) +
            " " +

             /**************************** Added This Line ****************************/
            (isDirtyRead ? withNolock : "") +
             /**************************** Added This Line ****************************/


            this.escape(destinationTableAlias) +
            " ON " +
            this.replacePropertyNames(condition + appendedCondition)
          );
        } else {
          // means many-to-many
          const junctionTableName = relation.junctionEntityMetadata!.tablePath;

          const junctionAlias = joinAttr.junctionAlias;
          let junctionCondition = "",
            destinationCondition = "";

          if (relation.isOwning) {
            junctionCondition = relation.joinColumns
              .map((joinColumn) => {
                // `post_category`.`postId` = `post`.`id`
                return (
                  junctionAlias +
                  "." +
                  joinColumn.propertyPath +
                  "=" +
                  parentAlias +
                  "." +
                  joinColumn.referencedColumn!.propertyPath
                );
              })
              .join(" AND ");

            destinationCondition = relation.inverseJoinColumns
              .map((joinColumn) => {
                // `category`.`id` = `post_category`.`categoryId`
                return (
                  destinationTableAlias +
                  "." +
                  joinColumn.referencedColumn!.propertyPath +
                  "=" +
                  junctionAlias +
                  "." +
                  joinColumn.propertyPath
                );
              })
              .join(" AND ");
          } else {
            junctionCondition = relation
              .inverseRelation!.inverseJoinColumns.map((joinColumn) => {
                // `post_category`.`categoryId` = `category`.`id`
                return (
                  junctionAlias +
                  "." +
                  joinColumn.propertyPath +
                  "=" +
                  parentAlias +
                  "." +
                  joinColumn.referencedColumn!.propertyPath
                );
              })
              .join(" AND ");

            destinationCondition = relation
              .inverseRelation!.joinColumns.map((joinColumn) => {
                // `post`.`id` = `post_category`.`postId`
                return (
                  destinationTableAlias +
                  "." +
                  joinColumn.referencedColumn!.propertyPath +
                  "=" +
                  junctionAlias +
                  "." +
                  joinColumn.propertyPath
                );
              })
              .join(" AND ");
          }

          return (
            " " +
            joinAttr.direction +
            " JOIN " +
            this.getTableName(junctionTableName) +
            " " +
            this.escape(junctionAlias) +

             /**************************** Added This Line ****************************/
            (isDirtyRead ? withNolock : "") +
             /**************************** Added This Line ****************************/


            " ON " +
            this.replacePropertyNames(junctionCondition) +
            " " +
            joinAttr.direction +
            " JOIN " +
            this.getTableName(destinationTableName) +
            " " +
            this.escape(destinationTableAlias) +
            " ON " +
            this.replacePropertyNames(destinationCondition + appendedCondition)
          );
        }
      });

      return joins.join(" ");
    }

icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
icecreamparlor added a commit to icecreamparlor/typeorm that referenced this issue Jan 5, 2022
typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: typeorm#4764
@icecreamparlor
Copy link
Contributor

I added some lines to fix this issue. I made a pull request like above.

@danaryu
Copy link

danaryu commented Jan 9, 2022

I faced same issue. Any updates ?

AlexMesser pushed a commit that referenced this issue Feb 12, 2022
* fix: add lock clause for MSSQL select with join clause

typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query.

Closes: #4764

* chore: add test cases

* chore: refactor method name, miscellaneous changes on createTableLockExpression
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants