finos / legend-studio

Legend Studio
https://legend.finos.org
Apache License 2.0
87 stars 114 forks source link

Bug: Self-join in database view causes application to hang #2388

Open akphi opened 1 year ago

akphi commented 1 year ago

Similar issues

How are you using Studio?

Legend Studio

Current and expected behavior

If we have a self-join used in a database view definition, the app hangs while trying to observe the graph

Steps to reproduce

Use the provided grammar, go back from text-mode to form-mode, try going back to text-mode

  1. You should not be able to due to some problem with transformer
  2. You should see Observing graph... being shown in the explorer tree

Model data

###Relational
Database model::DB
(
  Table MyTable
  (
    ID INTEGER,
    PARENT_ID INTEGER,
    NODE_NAME VARCHAR(255) PRIMARY KEY,
    LEVEL INTEGER PRIMARY KEY
  )

  View MyView
  (
    LEVEL_10: [model::DB]@MyJoin | {target}.NODE_NAME
  )

  Join MyJoin(MyTable.PARENT_ID = {target}.ID and {target}.LEVEL = 10)
)

Environment

No response

Possible solution and workaround

No response

Contribution

akphi commented 1 year ago

After a bit of digging, it seems like Studio graph builder is in sync with engine compiler, both will pass the code above, however, it will fail in Pure IDE

###Relational
Database model::DB
(
  Table MyTable
  (
    ID INTEGER,
    PARENT_ID INTEGER,
    NODE_NAME VARCHAR(255) PRIMARY KEY,
    LEVEL INTEGER PRIMARY KEY
  )

  View MyView
  (
    LEVEL_10: [model::DB]@MyJoin | {target}.NODE_NAME
  )

  Join MyJoin(MyTable.PARENT_ID = {target}.ID and {target}.LEVEL = 10)
)

I traced the logic of compilation in Studio and Engine and realize that, other than the processing DB join flow, if the self-join appears in an ElementWithJoins operation, we will not properly fill out its alias and even checking the `column

https://github.com/finos/legend-engine/blob/9c18f50675620dca15d0516c6844b8bac3187a2e/legend-engine-xt-relationalStore-grammar/src/main/java/org/finos/legend/engine/language/pure/compiler/toPureGraph/HelperRelationalBuilder.java#L803-L808

I then tried to test my hypothesis further with property mapping and realized the same problem for the following case

###Relational
Database model::DB
(
  Table MyTable
  (
    ID INTEGER,
    PARENT_ID INTEGER,
    NODE_NAME VARCHAR(255) PRIMARY KEY,
    LEVEL INTEGER PRIMARY KEY
  )

  Join MyJoin(MyTable.PARENT_ID = {target}.ID and {target}.LEVEL = 10)
)

###Pure
Class model::Node {
   name: String[1];
}

###Mapping
Mapping model::MyMapping (
   model::Node: Relational {
      ~mainTable [model::DB]MyTable
      name: [model::DB]@MyJoin | {target}.NODE_NAME
   }
)

... we can change {target}.NODE_NAME into anything and it will pass compilation


The workaround is to avoid using {target} syntax and just assign a table there

akphi commented 1 year ago

Also, it seems we don't validate the column at the end of the join expression, for example

###Relational
Database model::DB
(
  Table MyTable
  (
    ID INTEGER,
    PARENT_ID INTEGER,
    NODE_NAME VARCHAR(255) PRIMARY KEY,
    LEVEL INTEGER PRIMARY KEY
  )

  Table MyTable2
  (
    ID INTEGER,
    NAME VARCHAR(255)
  )

  Join MyJoin(MyTable.PARENT_ID = {target}.ID and {target}.LEVEL = 10)
)

###Pure
Class model::Node
{
  name: String[1];
}

###Mapping
Mapping model::MyMapping
(
  model::Node: Relational
  {
    ~mainTable [model::DB]MyTable
    name: [model::DB]@MyJoin | MyTable2.NAME
  }
)

^ we cannot navigate to Table2 via MyJoin and yet, we're using a column from Table2 :(

akphi commented 1 year ago
###Relational
Database test::DB
(
  Table PersonTable
  (
    id INTEGER PRIMARY KEY,
    manager_id INTEGER,
    name VARCHAR(200)
  )

  Join MySelfJoin(PersonTable.id = {target}.manager_id)
)

###Pure
Class test::Person
{
  name: String[1];
}

###Mapping
Mapping test::MyMapping
(
  *test::Person: Relational
  {
    ~primaryKey
    (
      [test::DB]PersonTable.id
    )
    ~mainTable [test::DB]PersonTable
    name: [test::DB]@MySelfJoin | [test::DB]PersonTable.name
  }
)

###Connection
RelationalDatabaseConnection test::Connection
{
  store: test::DB;
  type: H2;
  specification: LocalH2
  {
    testDataSetupSqls: [
      'Drop table if exists PersonTable',
      'Create Table PersonTable(id INT, manager_id INT, name VARCHAR(200))',
      'Insert into PersonTable (id, manager_id, name) values (1, 2, \'Jack\'), (2, 3, \'John\'), (3, 1, \'Peter\')'
      ];
  };
  auth: DefaultH2;
}

###Runtime
Runtime test::MyRuntime
{
  mappings:
  [
    test::MyMapping
  ];
  connections:
  [
    test::DB:
    [
      connection_1: test::Connection
    ]
  ];
}

select "persontable_1".name as "Name" from PersonTable as "root" left outer join PersonTable as "persontable_1" on ("root".id = "persontable_1".manager_id)

akphi commented 1 year ago

Just an update, we will stop working on this for now. engine will need to take care of this first, then we will handle this on our end