fix(typeorm,#895): Wrap all OR and AND expressions in brackets

* This ensures that groupings are correct
doug-martin · Feb 26, 2021 · 838ab16 · 838ab16
1 parent 07d9981
commit 838ab16
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 8 deletions.
diff --git a/packages/query-typeorm/__tests__/query/where.builder.spec.ts b/packages/query-typeorm/__tests__/query/where.builder.spec.ts
@@ -65,7 +65,7 @@ describe('WhereBuilder', (): void => {
             { numberType: { lte: 40 } },
           ],
         },
-        ` WHERE (("TestEntity"."number_type" > ?)) AND (("TestEntity"."number_type" < ?)) AND (("TestEntity"."number_type" >= ?)) AND (("TestEntity"."number_type" <= ?))`,
+        ` WHERE ((("TestEntity"."number_type" > ?)) AND (("TestEntity"."number_type" < ?)) AND (("TestEntity"."number_type" >= ?)) AND (("TestEntity"."number_type" <= ?)))`,
         [10, 20, 30, 40],
       );
     });
@@ -78,7 +78,7 @@ describe('WhereBuilder', (): void => {
             { numberType: { lt: 20 }, stringType: { like: '%bar' } },
           ],
         },
-        ` WHERE (("TestEntity"."number_type" > ?) AND ("TestEntity"."string_type" LIKE ?)) AND (("TestEntity"."number_type" < ?) AND ("TestEntity"."string_type" LIKE ?))`,
+        ` WHERE ((("TestEntity"."number_type" > ?) AND ("TestEntity"."string_type" LIKE ?)) AND (("TestEntity"."number_type" < ?) AND ("TestEntity"."string_type" LIKE ?)))`,
         [10, 'foo%', 20, '%bar'],
       );
     });
@@ -91,10 +91,21 @@ describe('WhereBuilder', (): void => {
             { or: [{ numberType: { gte: 30 } }, { numberType: { lte: 40 } }] },
           ],
         },
-        ` WHERE ((("TestEntity"."number_type" > ?)) OR (("TestEntity"."number_type" < ?))) AND ((("TestEntity"."number_type" >= ?)) OR (("TestEntity"."number_type" <= ?)))`,
+        ` WHERE ((((("TestEntity"."number_type" > ?)) OR (("TestEntity"."number_type" < ?)))) AND (((("TestEntity"."number_type" >= ?)) OR (("TestEntity"."number_type" <= ?)))))`,
         [10, 20, 30, 40],
       );
     });
+
+    it('should properly group AND with a sibling field comparison', (): void => {
+      assertSQL(
+        {
+          and: [{ numberType: { gt: 2 } }, { numberType: { lt: 10 } }],
+          stringType: { eq: 'foo' },
+        },
+        ` WHERE ((("TestEntity"."number_type" > ?)) AND (("TestEntity"."number_type" < ?))) AND ("TestEntity"."string_type" = ?)`,
+        [2, 10, 'foo'],
+      );
+    });
   });
 
   describe('or', (): void => {
@@ -108,7 +119,7 @@ describe('WhereBuilder', (): void => {
             { numberType: { lte: 40 } },
           ],
         },
-        ` WHERE (("TestEntity"."number_type" > ?)) OR (("TestEntity"."number_type" < ?)) OR (("TestEntity"."number_type" >= ?)) OR (("TestEntity"."number_type" <= ?))`,
+        ` WHERE ((("TestEntity"."number_type" > ?)) OR (("TestEntity"."number_type" < ?)) OR (("TestEntity"."number_type" >= ?)) OR (("TestEntity"."number_type" <= ?)))`,
         [10, 20, 30, 40],
       );
     });
@@ -121,7 +132,7 @@ describe('WhereBuilder', (): void => {
             { numberType: { lt: 20 }, stringType: { like: '%bar' } },
           ],
         },
-        ` WHERE (("TestEntity"."number_type" > ?) AND ("TestEntity"."string_type" LIKE ?)) OR (("TestEntity"."number_type" < ?) AND ("TestEntity"."string_type" LIKE ?))`,
+        ` WHERE ((("TestEntity"."number_type" > ?) AND ("TestEntity"."string_type" LIKE ?)) OR (("TestEntity"."number_type" < ?) AND ("TestEntity"."string_type" LIKE ?)))`,
         [10, 'foo%', 20, '%bar'],
       );
     });
@@ -134,9 +145,20 @@ describe('WhereBuilder', (): void => {
             { and: [{ numberType: { gte: 30 } }, { numberType: { lte: 40 } }] },
           ],
         },
-        ` WHERE ((("TestEntity"."number_type" > ?)) AND (("TestEntity"."number_type" < ?))) OR ((("TestEntity"."number_type" >= ?)) AND (("TestEntity"."number_type" <= ?)))`,
+        ` WHERE ((((("TestEntity"."number_type" > ?)) AND (("TestEntity"."number_type" < ?)))) OR (((("TestEntity"."number_type" >= ?)) AND (("TestEntity"."number_type" <= ?)))))`,
         [10, 20, 30, 40],
       );
     });
+
+    it('should properly group OR with a sibling field comparison', (): void => {
+      assertSQL(
+        {
+          or: [{ numberType: { eq: 2 } }, { numberType: { gt: 10 } }],
+          stringType: { eq: 'foo' },
+        },
+        ` WHERE ((("TestEntity"."number_type" = ?)) OR (("TestEntity"."number_type" > ?))) AND ("TestEntity"."string_type" = ?)`,
+        [2, 10, 'foo'],
+      );
+    });
   });
 });
diff --git a/packages/query-typeorm/src/query/where.builder.ts b/packages/query-typeorm/src/query/where.builder.ts
@@ -44,7 +44,9 @@ export class WhereBuilder<Entity> {
     relationNames: string[],
     alias?: string,
   ): Where {
-    return filters.reduce((w, f) => w.andWhere(this.createBrackets(f, relationNames, alias)), where);
+    return where.andWhere(
+      new Brackets((qb) => filters.reduce((w, f) => qb.andWhere(this.createBrackets(f, relationNames, alias)), qb)),
+    );
   }
 
   /**
@@ -60,7 +62,9 @@ export class WhereBuilder<Entity> {
     relationNames: string[],
     alias?: string,
   ): Where {
-    return filter.reduce((w, f) => where.orWhere(this.createBrackets(f, relationNames, alias)), where);
+    return where.andWhere(
+      new Brackets((qb) => filter.reduce((w, f) => qb.orWhere(this.createBrackets(f, relationNames, alias)), qb)),
+    );
   }
 
   /**