fix (client): avoid duplicate rows when including several relations (#…

…1026) This PR fixes a bug in the DAL that caused rows to be included several times when including multiple relations. The issue was reported on Discord: https://discord.com/channels/933657521581858818/1210387734234005614
electric-sql · Mar 5, 2024 · c30516e · c30516e
1 parent 85daa6e
commit c30516e
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 16 deletions.
diff --git a/.changeset/stale-suns-worry.md b/.changeset/stale-suns-worry.md
@@ -0,0 +1,5 @@
+---
+"electric-sql": patch
+---
+
+Fix issue with duplicate rows when including several relations.
diff --git a/clients/typescript/src/client/model/table.ts b/clients/typescript/src/client/model/table.ts
@@ -666,7 +666,7 @@ export class Table<
     relationType: Arity,
     includeArg: true | FindInput<any, any, any, any, any>,
     db: DB,
-    onResult: (joinedRows: Kind<GetPayload, T>[]) => void,
+    onResult: () => void,
     onError: (err: any) => void
   ) {
     const otherTable = this._tables.get(relatedTable)!
@@ -687,15 +687,16 @@ export class Table<
       (relatedRows: object[]) => {
         // Now, join the original `rows` with the `relatedRows`
         // where `row.fromField == relatedRow.toField`
-        const join = this.joinObjects(
+        // (this mutates the original rows)
+        this.joinObjects(
           rows,
           relatedRows,
           fromField,
           toField,
           relationField,
           relationType
         ) as Kind<GetPayload, T>[]
-        onResult(join)
+        onResult()
       },
       onError
     )
@@ -706,11 +707,11 @@ export class Table<
     relation: Relation,
     includeArg: boolean | FindInput<any, any, any, any, any>,
     db: DB,
-    onResult: (rows: Kind<GetPayload, T>[]) => void,
+    onResult: () => void,
     onError: (err: any) => void
   ) {
     if (includeArg === false) {
-      return onResult([])
+      return onResult()
     } else if (relation.isIncomingRelation()) {
       // incoming relation from the `fromField` in the other table
       // to the `toField` in this table
@@ -771,9 +772,6 @@ export class Table<
       return onResult(rows)
     else {
       const relationFields = Object.keys(include)
-      let includedRows: Kind<GetPayload, T>[] = []
-      // TODO: everywhere we use forEachCont we probably don't need continuation passing style!
-      //       so try to remove it there and then rename this one to `forEachCont`
       forEach(
         (relationField: string, cont: () => void) => {
           if (
@@ -796,22 +794,20 @@ export class Table<
             relationName
           )
 
+          // `fetchInclude` mutates the `rows` to include the related objects
           this.fetchInclude(
             rows,
             relation,
             include[relationField],
             db,
-            (fetchedRows) => {
-              includedRows = includedRows.concat(fetchedRows)
-              cont()
-            },
+            cont,
             onError
           )
         },
         relationFields,
         () => {
           // once the loop finished, call `onResult`
-          onResult(includedRows)
+          onResult(rows)
         }
       )
     }

diff --git a/clients/typescript/test/client/model/table.test.ts b/clients/typescript/test/client/model/table.test.ts
@@ -599,11 +599,9 @@ test.serial(
   'findMany can fetch related objects based on incoming FK of one-to-many relation',
   async (t) => {
     const res = await userTable.findMany({
-      where: {
-        id: 1,
-      },
       include: {
         posts: true,
+        profile: true,
       },
     })
 
@@ -612,6 +610,10 @@ test.serial(
         ...author1,
         posts: [post1, post2],
       },
+      {
+        ...author2,
+        posts: [post3],
+      },
     ])
   }
 )