Repository: datnguye/dbterd (language: python)

Issue:
Reverse relationships and ghost columns
**Describe the bug**
Say a relationship is defined in schema.yml on model X referencing model Y. However, sometimes in the output.dbml, the relationship is defined as Y => X, rather than X => Y. Furthermore, if the foreign_key column name on X is a different name from the primary_key column name on Y, then those are swapped as well. This also causes "ghost columns" to appear in the involved tables. For example, if the foreign_key column on model X is 'F', and the primary_key on model Y is 'P', then the relationship will sometimes be defined as: X[P] => Y[F]. This creates a ghost column 'F' on table Y, and a ghost column 'P' on table X.

**To Reproduce**
Steps to reproduce the behavior:
This behavior is random. See "additional context" for discovery

**Expected behavior**
The relationship in output.dbml reflects the correct column names and direction of the relation.

**Desktop (please complete the following information):**
 - OS: mac
 - Version dbterd v1.2.0

**Additional context**
I dug into the dbterd source code a bit and believe the problem is related to the line of code `table_map=manifest.parent_map[x]` in the `test_relationship.py`. It appears that this code expects the first element in the pair of the parent_map to be the foreign model, and the second element in the pair is the "this" model. However, looking in my manifest.json, it looks like DBT makes no such guarantees; as far as its concerned it is an unordered pair, and the order is likely some result of the order of each model in the DAG that it was compiled.

So, I think there needs to be some small logic that determines if the pair at `manifest.parent_map[x]` needs to be swapped before it is assigned to `table_map`.



Discussion / hints:
I did a test, and was able to solve the problem by creating a function

```
    def get_table_map(node):
        refs_arr = node.depends_on.nodes 
        foreign_model = node.test_metadata.kwargs['to']
        if f"'{refs_arr[1].split('.')[-1]}'" in foreign_model:
            refs_arr = [refs_arr[1],refs_arr[0]]
        return refs_arr
```

then replacing the call to parent_map with:
```
table_map=get_table_map(manifest.nodes[x]),
```

You'll note that I was able to find the same pair on `node.depends_on.nodes` instead of going through `parent_map`, and that pair does seem to be in reliable order, but kept the swap logic in the function because a) even though I'm seeing it always in order, it can't hurt to ensure in case its still not guaranteed, and b) if there's a reason you're referencing parent_map instead of depends_on (maybe `depends_on.nodes` isn't always present I'm not sure), you can add the `parent_map[x]` as another function parameter and it still works.

There's probably opportunity to make that function cleaner, but from a logic standpoint, it does appear to solve the bug.

Thanks @danny-chain for raising the issue and its solution as well. I will work on it this weekend, but feel free to create PR while awaiting. Cheers

Fix patch:
diff --git a/docs/index.md b/docs/index.md
index a3bcb86..3ce3c94 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -1,6 +1,6 @@
 # dbterd
 
-CLI to generate Diagram-as-a-code file ([DBML](https://dbdiagram.io/d), [Mermaid](https://mermaid-js.github.io/mermaid-live-editor/), [PlantUML](https://plantuml.com/ie-diagram), [GraphViz](https://graphviz.org/), [D2](https://d2lang.com/)) from dbt artifact files (required: `manifest.json`, `catalog.json`)
+CLI to generate Diagram-as-a-code file ([DBML](https://dbdiagram.io/d), [Mermaid](https://mermaid-js.github.io/mermaid-live-editor/), [PlantUML](https://plantuml.com/ie-diagram), [GraphViz](https://graphviz.org/), [D2](https://d2lang.com/)) from dbt artifact files (required: [![dbt](https://img.shields.io/badge/manifest.json-upto--v9-3776AB.svg?style=flat&logo=dbt&logoColor=orange)](https://schemas.getdbt.com/) [![dbt](https://img.shields.io/badge/catalog.json-upto--v1-3776AB.svg?style=flat&logo=dbt&logoColor=orange)](https://schemas.getdbt.com/))
 
 [![PyPI version](https://badge.fury.io/py/dbterd.svg)](https://pypi.org/project/dbterd/)
 ![python-cli](https://img.shields.io/badge/CLI-Python-FFCE3E?labelColor=14354C&logo=python&logoColor=white)


Test patch:
diff --git a/dbterd/adapters/algos/test_relationship.py b/dbterd/adapters/algos/test_relationship.py
index d4c8c1c..45f1fd0 100644
--- a/dbterd/adapters/algos/test_relationship.py
+++ b/dbterd/adapters/algos/test_relationship.py
@@ -55,43 +55,30 @@ def get_relationships(manifest, **kwargs):
 
     Args:
         manifest (dict): Manifest json
-        kwargs.algo (str): |
-            Algorithm name and optional rules.
-            Smaples:
-            - test_relationship
-            - test_relationship:(name:relationship|c_from:column_name|c_to:field)
-            - test_relationship:(name:foreign_key|c_from:fk_column_name|c_to:pk_column_name)
 
     Returns:
         List[Ref]: List of parsed relationship
     """
-    algo_parts = (kwargs.get("algo") or "").replace(" ", "").split(":", 1)
-    rules, _ = (
-        algo_parts[1] if len(algo_parts) > 1 else DEFAULT_ALGO_RULE,
-        algo_parts[0],
-    )
-    rules = rules[1:-1]  # remove brackets
-    rules = dict(arg.split(":") for arg in rules.split("|"))
-
+    rule = get_algo_rule(**kwargs)
     refs = [
         Ref(
             name=x,
-            table_map=manifest.parent_map[x],
+            table_map=get_table_map(test_node=manifest.nodes[x], **kwargs),
             column_map=[
                 str(
-                    manifest.nodes[x].test_metadata.kwargs.get(rules.get("c_to"))
+                    manifest.nodes[x].test_metadata.kwargs.get(rule.get("c_to"))
                     or "_and_".join(
                         manifest.nodes[x].test_metadata.kwargs.get(
-                            f'{rules.get("c_to")}s', "unknown"
+                            f'{rule.get("c_to")}s', "unknown"
                         )
                     )
                 ).lower(),
                 str(
                     manifest.nodes[x].test_metadata.kwargs.get("column_name")
-                    or manifest.nodes[x].test_metadata.kwargs.get(rules.get("c_from"))
+                    or manifest.nodes[x].test_metadata.kwargs.get(rule.get("c_from"))
                     or "_and_".join(
                         manifest.nodes[x].test_metadata.kwargs.get(
-                            f'{rules.get("c_from")}s', "unknown"
+                            f'{rule.get("c_from")}s', "unknown"
                         )
                     )
                 ).lower(),
@@ -103,7 +90,7 @@ def get_relationships(manifest, **kwargs):
         for x in manifest.nodes
         if (
             x.startswith("test")
-            and rules.get("name").lower() in x.lower()
+            and rule.get("name").lower() in x.lower()
             and manifest.nodes[x].meta.get(TEST_META_IGNORE_IN_ERD, "0") == "0"
         )
     ]
@@ -121,6 +108,59 @@ def get_relationships(manifest, **kwargs):
     return []
 
 
+def get_algo_rule(**kwargs):
+    """Extract rule from the --algo option
+
+    Args:
+        kwargs.algo (str): |
+            Algorithm name and optional rules.
+            Samples:
+            - test_relationship
+            - test_relationship:(name:relationship|c_from:column_name|c_to:field)
+            - test_relationship:(name:foreign_key|c_from:fk_column_name|c_to:pk_column_name)
+            - test_relationship:(
+                name:foreign_key|
+                c_from:fk_column_name|
+                c_to:pk_column_name|
+                t_to:pk_table_name
+            )
+
+    Returns:
+        dict: Rule object (
+            name [default 'relationship', use contains],
+            c_from [default 'column_name'],
+            c_to [default 'field'],
+            t_to [default 'to']
+        )
+    """
+    algo_parts = (kwargs.get("algo") or "").replace(" ", "").split(":", 1)
+    rules, _ = (
+        algo_parts[1] if len(algo_parts) > 1 else DEFAULT_ALGO_RULE,
+        algo_parts[0],
+    )
+    rules = rules[1:-1]  # remove brackets
+    rules = dict(arg.split(":") for arg in rules.split("|"))
+    return rules
+
+
+def get_table_map(test_node, **kwargs):
+    """Get the table map with order of [to, from] guaranteed
+
+    Args:
+        test_node (dict): Manifest Test node
+
+    Returns:
+        list: [to model, from model]
+    """
+    map = test_node.depends_on.get("nodes", [])
+    rule = get_algo_rule(**kwargs)
+    to_model = str(test_node.test_metadata.kwargs.get(rule.get("t_to", "to"), {}))
+    if f'("{map[1].split(".")[-1]}")'.lower() in to_model.replace("'", '"').lower():
+        return [map[1], map[0]]
+
+    return map
+
+
 def get_relationship_type(meta: str) -> str:
     """Get short form of the relationship type configured in test meta
 
diff --git a/tests/unit/adapters/algos/test_test_relationship.py b/tests/unit/adapters/algos/test_test_relationship.py
index 1965c51..5a3b7e6 100644
--- a/tests/unit/adapters/algos/test_test_relationship.py
+++ b/tests/unit/adapters/algos/test_test_relationship.py
@@ -1,4 +1,4 @@
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from unittest import mock
 from unittest.mock import MagicMock
 
@@ -44,6 +44,7 @@ class ManifestNode:
     raw_sql: str = ""
     database: str = ""
     schema_: str = ""
+    depends_on: dict = field(default_factory=dict)
 
 
 @dataclass
@@ -55,58 +56,70 @@ class ManifestNodeColumn:
 
 @dataclass
 class DummyManifestRel:
-    parent_map = dict(
-        {
-            "test.dbt_resto.relationships_table1": ["table2", "table1"],
-            "test.dbt_resto.relationships_table2": ["table2", "table1"],
-            "test.dbt_resto.relationships_table3": ["tabley", "tablex"],
-            "test.dbt_resto.relationships_tablex": ["y", "x"],
-            "test.dbt_resto.foreign_key_table1": ["table2", "table1"],
-            "test.dbt_resto.relationships_table4": ["table-m2", "table-m1"],
-        }
-    )
     nodes = {
         "test.dbt_resto.relationships_table1": ManifestNode(
             test_metadata=ManifestNodeTestMetaData(
-                kwargs={"column_name": "f1", "field": "f2"}
+                kwargs={"column_name": "f1", "field": "f2", "to": "ref('table2')"}
             ),
             meta={},
             columns={},
+            depends_on=dict(nodes=["model.dbt_resto.table2", "model.dbt_resto.table1"]),
         ),
         "test.dbt_resto.relationships_table2": ManifestNode(
             test_metadata=ManifestNodeTestMetaData(
-                kwargs={"column_name": "f1", "field": "f2"}
+                kwargs={"column_name": "f1", "field": "f2", "to": "ref('table2')"}
             ),
             meta={},
             columns={},
+            depends_on=dict(nodes=["model.dbt_resto.table2", "model.dbt_resto.table1"]),
         ),
         "test.dbt_resto.relationships_table3": ManifestNode(
             test_metadata=ManifestNodeTestMetaData(
-                kwargs={"column_name": "f1", "field": "f2"}
+                kwargs={"column_name": "f1", "field": "f2", "to": "ref('tabley')"}
             ),
             meta={},
             columns={},
+            depends_on=dict(nodes=["model.dbt_resto.tabley", "model.dbt_resto.tablex"]),
         ),
         "test.dbt_resto.relationships_tablex": ManifestNode(
             test_metadata=ManifestNodeTestMetaData(
-                kwargs={"column_name": "x", "field": "y"}
+                kwargs={"column_name": "x", "field": "y", "to": "ref('y')"}
             ),
             meta={"ignore_in_erd": 1},
             columns={},
+            depends_on=dict(nodes=["model.dbt_resto.y", "model.dbt_resto.x"]),
         ),
         "test.dbt_resto.foreign_key_table1": ManifestNode(
             test_metadata=ManifestNodeTestMetaData(
-                kwargs={"column_name": "f1", "pk_column_name": "f2"}
+                kwargs={
+                    "column_name": "f1",
+                    "pk_column_name": "f2",
+                    "pk_table_name": "ref('table2')",
+                }
             ),
             meta={},
             columns={},
+            depends_on=dict(nodes=["model.dbt_resto.table2", "model.dbt_resto.table1"]),
         ),
         "test.dbt_resto.relationships_table4": ManifestNode(
             test_metadata=ManifestNodeTestMetaData(
-                kwargs={"column_name": "f1", "field": "f2"}
+                kwargs={"column_name": "f1", "field": "f2", "to": "ref('table-m2')"}
             ),
             meta={"relationship_type": "one-to-one"},
             columns={},
+            depends_on=dict(
+                nodes=["model.dbt_resto.table-m2", "model.dbt_resto.table-m1"]
+            ),
+        ),
+        "test.dbt_resto.relationships_table1_reverse": ManifestNode(
+            test_metadata=ManifestNodeTestMetaData(
+                kwargs={"column_name": "f1", "field": "f2", "to": "ref('table-r2')"}
+            ),
+            meta={},
+            columns={},
+            depends_on=dict(
+                nodes=["model.dbt_resto.table-r1", "model.dbt_resto.table-r2"]
+            ),
         ),
     }
 
@@ -269,20 +282,31 @@ def test_get_compiled(self, manifest, expected):
                 [
                     Ref(
                         name="test.dbt_resto.relationships_table1",
-                        table_map=["table2", "table1"],
+                        table_map=["model.dbt_resto.table2", "model.dbt_resto.table1"],
                         column_map=["f2", "f1"],
                     ),
                     Ref(
                         name="test.dbt_resto.relationships_table3",
-                        table_map=["tabley", "tablex"],
+                        table_map=["model.dbt_resto.tabley", "model.dbt_resto.tablex"],
                         column_map=["f2", "f1"],
                     ),
                     Ref(
                         name="test.dbt_resto.relationships_table4",
-                        table_map=["table-m2", "table-m1"],
+                        table_map=[
+                            "model.dbt_resto.table-m2",
+                            "model.dbt_resto.table-m1",
+                        ],
                         column_map=["f2", "f1"],
                         type="11",
                     ),
+                    Ref(
+                        name="test.dbt_resto.relationships_table1_reverse",
+                        table_map=[
+                            "model.dbt_resto.table-r2",
+                            "model.dbt_resto.table-r1",
+                        ],
+                        column_map=["f2", "f1"],
+                    ),
                 ],
             ),
             (
@@ -291,12 +315,12 @@ def test_get_compiled(self, manifest, expected):
                 [
                     Ref(
                         name="test.dbt_resto.foreign_key_table1",
-                        table_map=["table2", "table1"],
+                        table_map=["model.dbt_resto.table2", "model.dbt_resto.table1"],
                         column_map=["f2", "f1"],
                     ),
                 ],
             ),
-            (MagicMock(return_value={"parent_map": [], "nodes": {}}), None, []),
+            (MagicMock(return_value={"depends_on": {}, "nodes": {}}), None, []),
         ],
     )
     def test_get_relationships(self, manifest, algorithm, expected):