informatics-isi-edu / ermrestjs

ERMrest client library in JavaScript
Apache License 2.0
4 stars 3 forks source link

Properly share path prefix while parsing facets #924

Closed RFSH closed 2 years ago

RFSH commented 2 years ago

Description

This PR is created to fix the correctness issue related to shared path prefix. The code wasn't properly sharing path for the request to fetch the facets (#896 ).

Examples

To make this easier to understand, assume the following ERD with the following source definitions on table schema:main:

ERD

{
   "path_to_A": {
     "source": [{"inbound": ["schema", "a_main_fk"]}, "RID"]
   },
   "path_to_B": {
      "source": [
        {"sourcekey": "path_to_A"},
        {"outbound": ["schema", "a_b_fk"]},
        "RID"
      ]
    },
   "path_to_C": {
      "source": [
        {"sourcekey": "path_to_A"},
        {"outbound": ["schema", "a_c_fk"]},
        "RID"
      ]
    }
}

Example 1

Navigating to a schema:main page with the following facet blob:

{
  "and": [
      {"sourcekey": "path_to_B", "choices": ["v1"]},
      {"sourcekey": "path_to_C", "choices": ["v2"]},
  ]
}

Should result in the following main request:

M:=s:main/
         M_P1:=(RID)=(A:fk_to_main)/(fk_to_B)=(B:RID)/RID=v1/$M/
         $M_P1/(fk_to_C)=(C:RID)/RID=v2/$M/
         F1:=left(fk_to_D)=(D:RID)/
         RID;M:=array(M:*),F1:=array(F1:*)

And the following request for facet B:

T:=s:main/
         T_P1:=(RID)=(A:fk_to_main)/(fk_to_C)=(C:RID)/RID=v2/$T/
         $T_P1/M:=(fk_to_B)=(B:RID)/
         F1:=left(fk_to_E)=(E:RID)/
         RID;M:=array(M:*),F1:=array(F1:*)

The existing code was adding a new join statement from table main to A for the path from main to B.

Example 2

The changes in this PR also handle the cases where a facet is superset of the other ones. For example:

{
  "and": [
      {"sourcekey": "path_to_A", "choices": ["v1"]},
      {"sourcekey": "path_to_C", "choices": ["v2"]},
  ]
}

Which will result in the following request for the main table:

M:=s:main/
         M_P1:=(RID)=(A:fk_to_main)/RID=v1/$M/
         $M_P1/(fk_to_C)=(C:RID)/RID=v2/$M/
         F1:=left(fk_to_D)=(D:RID)/
         RID;M:=array(M:*),F1:=array(F1:*)

And the following request for first facet (A):

T:=s:main/
         M:=(RID)=(A:fk_to_main)/(fk_to_C)=(C:RID)/RID=v2/$T/
         $M/
         F1:=left(fk_to_E)=(E:RID)/
         RID;M:=array(M:*),F1:=array(F1:*)

The existing code was adding a new join statement from table main to C while the new code will make sure it's using the table instance that is already created. Also what's special in this case is the alias M. Alias M is documented as the alias that is used for the projected result and that's why the new code is ensuring a new alias is not created and we're using the M alias.

Further changes

These request can can be optimized by removing the path reset ($T) that are added after each facet, but that should be part of another PR as it will cause other issues (it will also require a lot of changes in tests).