feat: Schema tree calculcation for circular input schemas (#79)

* circular path schema calcaulation

* updated error message with input

* removed type from fixture

* test failure fix

* added test cases for loop in join paths

* bumped package verisons

* lint error fix

* removed uneeded code

* test failure fix

Author: zaidjan-devrev

meerkat-browser/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@devrev/meerkat-browser",
-  "version": "0.0.72",
+  "version": "0.0.73",
   "dependencies": {
     "@swc/helpers": "~0.5.0",
     "@devrev/meerkat-core": "*",

meerkat-core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@devrev/meerkat-core",
-  "version": "0.0.74",
+  "version": "0.0.75",
   "dependencies": {
     "@swc/helpers": "~0.5.0"
   },

meerkat-core/src/joins/joins.spec.ts

@@ -1,5 +1,5 @@
 import {
-  checkLoopInGraph,
+  checkLoopInJoinPath,
   createDirectedGraph,
   generateSqlQuery,
 } from './joins';
@@ -60,32 +60,6 @@ describe('Table schema functions', () => {
     });
   });
 
-  it('should correctly identify if a loop exists in the graph', () => {
-    const graph = {
-      table1: {
-        table2: {
-          field1: 'table2.field3 = table1.field4',
-        },
-        table3: {
-          field2: 'table3.field5 = table1.field2',
-        },
-      },
-      table2: {
-        table3: {
-          field3: 'table3.field4 = table2.field3',
-        },
-      },
-      table3: {
-        table1: {
-          field5: 'table1.field1 = table3.field2',
-        },
-      },
-    };
-    const hasLoop = checkLoopInGraph(graph);
-
-    expect(hasLoop).toBe(true);
-  });
-
   it('should correctly generate a SQL query from the provided join path, table schema SQL map, and directed graph', () => {
     const joinPaths = [
       [
@@ -113,21 +87,34 @@ describe('Table schema functions', () => {
     );
   });
 
-  it('should throw an error when a cycle exists in checkLoopInGraph', () => {
-    const graph = {
-      node1: { node2: { id: 'node1.id = node2.id' } },
-      node2: { node3: { id: 'node2.id = node3.id ' } },
-      node3: { node1: { id: 'node3.id = node1.id' } },
-    };
-    const output = checkLoopInGraph(graph);
-    expect(output).toBe(true);
-  });
-
-  it('checkLoopInGraph should return false for disconnected graph', () => {
-    const graph = {
-      node1: { node2: { id: 'node1.id = node2.id ' } },
-      node3: { node4: { id: 'node3.id = node4.id ' } },
-    };
-    expect(checkLoopInGraph(graph)).toBe(false);
-  });
+  describe('checkLoopInJoinPath', () => {
+    it('should return false if there is no loop in the join path', () => {
+      const joinPath = [
+        [
+          { left: 'table1', right: 'table2', on: 'id' },
+          { left: 'table2', right: 'table3', on: 'id' },
+        ],
+      ];
+      expect(checkLoopInJoinPath(joinPath)).toBe(false);
+    })
+    it('should return true if there is a loop in the join path', () => {
+      const joinPath = [
+        [
+          { left: 'table1', right: 'table2', on: 'id' },
+          { left: 'table2', right: 'table3', on: 'id' },
+          { left: 'table3', right: 'table1', on: 'id' },
+        ],
+      ];
+      expect(checkLoopInJoinPath(joinPath)).toBe(true);
+    })
+    it('should return false for single node', () => {
+      const joinPath = [
+        [
+          { left: 'table1', },
+          { left: 'table1' },
+        ],
+      ];
+      expect(checkLoopInJoinPath(joinPath)).toBe(false);
+    })
+  })
 });

meerkat-core/src/joins/joins.ts

@@ -57,11 +57,9 @@ export function generateSqlQuery(
       // If visitedFrom is undefined, this is the first visit to the node
       visitedNodes.set(currentEdge.right, currentEdge);
 
-      query += ` LEFT JOIN (${tableSchemaSqlMap[currentEdge.right]}) AS ${
-        currentEdge.right
-      }  ON ${
-        directedGraph[currentEdge.left][currentEdge.right][currentEdge.on]
-      }`;
+      query += ` LEFT JOIN (${tableSchemaSqlMap[currentEdge.right]}) AS ${currentEdge.right
+        }  ON ${directedGraph[currentEdge.left][currentEdge.right][currentEdge.on]
+        }`;
     }
   }
 
@@ -152,38 +150,23 @@ export const createDirectedGraph = (
   return directedGraph;
 };
 
-function DFS(
-  graph: any,
-  node: string,
-  visited: Set<string>,
-  recStack: Set<string>
-): boolean {
-  visited.add(node);
-  recStack.add(node);
-
-  for (const neighbor in graph[node]) {
-    if (!visited.has(neighbor) && DFS(graph, neighbor, visited, recStack)) {
-      return true;
-    } else if (recStack.has(neighbor)) {
-      return true;
-    }
-  }
-
-  recStack.delete(node);
-  return false;
-}
 
-export function checkLoopInGraph(graph: any): boolean {
-  const visited = new Set<string>();
-  const recStack = new Set<string>();
-
-  for (const node in graph) {
-    if (DFS(graph, node, visited, recStack)) {
-      return true;
+export const checkLoopInJoinPath = (joinPath: JoinPath[]) => {
+  for (let i = 0; i < joinPath.length; i++) {
+    const visitedNodes = new Set<string>();
+    const currentJoinPath = joinPath[i];
+    visitedNodes.add(currentJoinPath[0].left);
+    for (let j = 0; j < currentJoinPath.length; j++) {
+      const currentEdge = currentJoinPath[j];
+      if (isJoinNode(currentEdge) && visitedNodes.has(currentEdge.right)) {
+        if (visitedNodes.has(currentEdge.right)) {
+          return true;
+        }
+        visitedNodes.add(currentEdge.right);
+      }
     }
   }
-
-  return false;
+  return false
 }
 
 export const getCombinedTableSchema = async (
@@ -202,9 +185,9 @@ export const getCombinedTableSchema = async (
   );
 
   const directedGraph = createDirectedGraph(tableSchema, tableSchemaSqlMap);
-  const hasLoop = checkLoopInGraph(directedGraph);
+  const hasLoop = checkLoopInJoinPath(cubeQuery.joinPaths || []);
   if (hasLoop) {
-    throw new Error('A loop was detected in the joins.');
+    throw new Error(`A loop was detected in the joins. ${JSON.stringify(cubeQuery.joinPaths || [])}`);
   }
 
   const baseSql = generateSqlQuery(