Skip to content

Commit

Permalink
Join recursive cte to base (#880)
Browse files Browse the repository at this point in the history 
* Join recursive cte to base

* Fix comment

* Add recurse inside optional test

* Remove TODO

* Lint

* Remove todo

* Add comment

* Fix mypy hint

* Update graphql_compiler/tests/integration_tests/test_backends_integration.py

Co-authored-by: Predrag Gruevski <[email protected]>

* Update graphql_compiler/tests/integration_tests/test_backends_integration.py

Co-authored-by: Predrag Gruevski <[email protected]>

* Remove autogenerated comment

* Add cautionary comment

* Offset test comments

* Add another integration test

* Lint

* Re-indent test

* Update graphql_compiler/compiler/emit_sql.py

Co-authored-by: Predrag Gruevski <[email protected]>

Co-authored-by: Predrag Gruevski <[email protected]>
  • Loading branch information
@bojanserafimov @obi1kenobi
bojanserafimov and obi1kenobi authored Jul 16, 2020
1 parent 129be64 commit 8d69f4e
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 81 deletions.
34 changes: 22 additions & 12 deletions graphql_compiler/compiler/emit_sql.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ def generate_subquery(self) -> str:
return alias


@dataclass
@dataclass(eq=False) # Setting eq=False allows the class to inherit id-based hash from object
class ColumnRouter:
"""Container for columns selected from a variety of selectables.
Expand Down Expand Up @@ -743,7 +743,7 @@ def __init__(self, sql_schema_info: SQLAlchemySchemaInfo, ir: IrAndMetadata):
self._relocate(ir.query_metadata_table.root_location)

# Mapping aliases to the column used to join into them.
self._came_from: Dict[Alias, Column] = {}
self._came_from: Dict[Union[Alias, ColumnRouter], Column] = {}

self._recurse_needs_cte: bool = False

Expand Down Expand Up @@ -830,6 +830,10 @@ def _join_to_parent_location(
# approach is incorrect because a mandatory edge traversal miss inside an optional
# scope is supposed to invalidate the whole result. However, with this solution the
# result will still appear.
# 3. You might think it's safe to INNER JOIN on recursive traversals in optional
# scope, since the recursion always contains the 0-th level, which is the vertex
# itself. However, when you join a selectable to itself, any rows with NULL value
# at the join key are dropped.
self._filters.append(
sqlalchemy.or_(
self._came_from[self._current_alias].isnot(None),
Expand Down Expand Up @@ -913,10 +917,13 @@ def _wrap_into_cte(self) -> None:
if self._recurse_needs_cte:
self._current_alias = self.get_query(extra_outputs).cte(recursive=False)
self._from_clause = self._current_alias

self._filters = [] # The filters are already included in the cte
self._aliases = {
alias_key: ColumnRouter(

# After creating this CTE, accessing a field of any of the currently marked
# locations should redirect to the column of the CTE that exposes that field.
# Here we replace all aliases with ColumnRouters pointing to the CTE.
old_to_new_alias = {
alias_value: ColumnRouter(
{
external_name: self._current_alias.c[internal_name]
for external_name, internal_name in column_mappings.get(
Expand All @@ -926,6 +933,14 @@ def _wrap_into_cte(self) -> None:
)
for alias_key, alias_value in self._aliases.items()
}
self._came_from = {
old_to_new_alias[alias]: old_to_new_alias[alias].c[came_from.name]
for alias, came_from in self._came_from.items()
}
self._aliases = {
alias_key: old_to_new_alias[alias] for alias_key, alias in self._aliases.items()
}

if not isinstance(self._current_location, Location):
raise AssertionError(
f"Attempted to wrap to CTE while the _current_location of was type "
Expand Down Expand Up @@ -1020,13 +1035,8 @@ def recurse(self, vertex_field: str, depth: int) -> None:
.where(base.c[CTE_DEPTH_NAME] < literal_depth)
)

# Instead of joining to the current _from_clause, we make this alias the _from_clause.
# If the existing _from_clause had any information in it, then the _current_alias would
# be a cte that contains all that information.
# TODO(bojanserafimov): If the existing _from_clause had no filters or traversals, but
# had an output, the output needs to be moved into the cte or
# we need to join to the _from_clause.
self._from_clause = self._current_alias
# TODO(bojanserafimov): This creates an unused alias if there's no tags or outputs so far
self._join_to_parent_location(previous_alias, primary_key, CTE_KEY_NAME, False)

def start_global_operations(self) -> None:
"""Execute a GlobalOperationsStart block."""
Expand Down
217 changes: 167 additions & 50 deletions graphql_compiler/tests/integration_tests/test_backends_integration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,29 +289,29 @@ def test_recurse(self) -> None:

# (query, expected_results) pairs. All of them running with the same parameters.
# The queries are ran in the order specified here.
queries = [
queries: List[Tuple[str, List[Dict[str, Any]]]] = [
# Query 1: Just the root
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
@output(out_name: "root_name")
}
}""",
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
@output(out_name: "root_name")
}
}""",
[{"root_name": "Animal 1"}],
),
# Query 2: Immediate children
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
name @output(out_name: "descendant_name")
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
name @output(out_name: "descendant_name")
}
}
}
}""",
}""",
[
{"descendant_name": "Animal 1"},
{"descendant_name": "Animal 2"},
Expand All @@ -321,16 +321,16 @@ def test_recurse(self) -> None:
# Query 3: Grandchildren
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
name @output(out_name: "descendant_name")
out_Animal_ParentOf {
name @output(out_name: "descendant_name")
}
}
}
}
}""",
}""",
[
{"descendant_name": "Animal 1"},
{"descendant_name": "Animal 2"},
Expand All @@ -341,18 +341,18 @@ def test_recurse(self) -> None:
# Query 4: Grand-grandchildren
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
out_Animal_ParentOf {
name @output(out_name: "descendant_name")
out_Animal_ParentOf {
name @output(out_name: "descendant_name")
}
}
}
}
}
}""",
}""",
[
{"descendant_name": "Animal 1"},
{"descendant_name": "Animal 2"},
Expand All @@ -363,14 +363,14 @@ def test_recurse(self) -> None:
# Query 5: Recurse depth 1
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf @recurse(depth: 1){
name @output(out_name: "descendant_name")
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf @recurse(depth: 1){
name @output(out_name: "descendant_name")
}
}
}
}""",
}""",
[
{"descendant_name": "Animal 1"}, # depth 0 match
{"descendant_name": "Animal 1"}, # depth 1 match
Expand All @@ -381,14 +381,14 @@ def test_recurse(self) -> None:
# Query 6: Recurse depth 2
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf @recurse(depth: 2){
name @output(out_name: "descendant_name")
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf @recurse(depth: 2){
name @output(out_name: "descendant_name")
}
}
}
}""",
}""",
[
{"descendant_name": "Animal 1"}, # depth 0 match
{"descendant_name": "Animal 1"}, # depth 1 match
Expand All @@ -403,14 +403,14 @@ def test_recurse(self) -> None:
# Query 7: Recurse depth 3
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf @recurse(depth: 3){
name @output(out_name: "descendant_name")
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf @recurse(depth: 3){
name @output(out_name: "descendant_name")
}
}
}
}""",
}""",
[
{"descendant_name": "Animal 1"}, # depth 0 match
{"descendant_name": "Animal 1"}, # depth 1 match
Expand All @@ -426,6 +426,123 @@ def test_recurse(self) -> None:
{"descendant_name": "Animal 4"}, # depth 3 match
],
),
# Query 8: Skip depth 0
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
out_Animal_ParentOf @recurse(depth: 2) {
name @output(out_name: "descendant_name")
}
}
}
}""",
[
{"descendant_name": "Animal 1"}, # depth 0 match
{"descendant_name": "Animal 2"}, # depth 0 match
{"descendant_name": "Animal 3"}, # depth 0 match
{"descendant_name": "Animal 1"}, # depth 1 match
{"descendant_name": "Animal 2"}, # depth 1 match
{"descendant_name": "Animal 3"}, # depth 1 match
{"descendant_name": "Animal 4"}, # depth 1 match
{"descendant_name": "Animal 1"}, # depth 2 match
{"descendant_name": "Animal 2"}, # depth 2 match
{"descendant_name": "Animal 3"}, # depth 2 match
{"descendant_name": "Animal 4"}, # depth 2 match
],
),
# Query 9: Output child name
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
name @output(out_name: "child_name")
out_Animal_ParentOf @recurse(depth: 2) {
name @output(out_name: "descendant_name")
}
}
}
}""",
[
{"child_name": "Animal 1", "descendant_name": "Animal 1"}, # depth 0 match
{"child_name": "Animal 2", "descendant_name": "Animal 2"}, # depth 0 match
{"child_name": "Animal 3", "descendant_name": "Animal 3"}, # depth 0 match
{"child_name": "Animal 1", "descendant_name": "Animal 1"}, # depth 1 match
{"child_name": "Animal 1", "descendant_name": "Animal 2"}, # depth 1 match
{"child_name": "Animal 1", "descendant_name": "Animal 3"}, # depth 1 match
{"child_name": "Animal 3", "descendant_name": "Animal 4"}, # depth 1 match
{"child_name": "Animal 1", "descendant_name": "Animal 1"}, # depth 2 match
{"child_name": "Animal 1", "descendant_name": "Animal 2"}, # depth 2 match
{"child_name": "Animal 1", "descendant_name": "Animal 3"}, # depth 2 match
{"child_name": "Animal 1", "descendant_name": "Animal 4"}, # depth 2 match
],
),
# Query 10: Recurse within optional scope. Animal_1 has no grandchildren from its
# child Animal_2, but since we use an @optional edge, Animal_2 should
# still appear in the result as a child of Animal_1. Here we are testing
# that the use of @recurse does not interfere with @optional semantics.
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
name @output(out_name: "child_name")
out_Animal_ParentOf @optional {
out_Animal_ParentOf @recurse(depth: 1){
name @output(out_name: "descendant_name")
}
}
}
}
}""",
[
{"child_name": "Animal 1", "descendant_name": "Animal 1"}, # depth 0 match
{"child_name": "Animal 1", "descendant_name": "Animal 2"}, # depth 0 match
{"child_name": "Animal 1", "descendant_name": "Animal 3"}, # depth 0 match
{"child_name": "Animal 2", "descendant_name": None}, # depth 0 match
{"child_name": "Animal 3", "descendant_name": "Animal 4"}, # depth 0 match
{"child_name": "Animal 1", "descendant_name": "Animal 1"}, # depth 1 match
{"child_name": "Animal 1", "descendant_name": "Animal 2"}, # depth 1 match
{"child_name": "Animal 1", "descendant_name": "Animal 3"}, # depth 1 match
{"child_name": "Animal 1", "descendant_name": "Animal 4"}, # depth 1 match
],
),
# Query 11: Same as query 10, but with additional traversal inside the @recurse.
(
"""
{
Animal {
name @filter(op_name: "=", value: ["$starting_animal_name"])
out_Animal_ParentOf {
name @output(out_name: "child")
out_Animal_ParentOf @optional {
out_Animal_ParentOf @recurse(depth: 1){
name @output(out_name: "descendant")
out_Animal_ParentOf {
name @output(out_name: "tiny_child")
}
}
}
}
}
}""",
[
{"child": "Animal 1", "descendant": "Animal 1", "tiny_child": "Animal 1"},
{"child": "Animal 1", "descendant": "Animal 1", "tiny_child": "Animal 2"},
{"child": "Animal 1", "descendant": "Animal 1", "tiny_child": "Animal 3"},
{"child": "Animal 1", "descendant": "Animal 3", "tiny_child": "Animal 4"},
{"child": "Animal 2", "descendant": None, "tiny_child": None},
{"child": "Animal 1", "descendant": "Animal 1", "tiny_child": "Animal 1"},
{"child": "Animal 1", "descendant": "Animal 1", "tiny_child": "Animal 2"},
{"child": "Animal 1", "descendant": "Animal 1", "tiny_child": "Animal 3"},
{"child": "Animal 1", "descendant": "Animal 3", "tiny_child": "Animal 4"},
],
),
]

# TODO(bojanserafimov): Only testing in MSSQL because none of our backends agree on recurse
Expand Down Expand Up @@ -713,7 +830,7 @@ def test_snapshot_graphql_schema_from_orientdb_schema(self):
"UniquelyIdentifiable": {"uuid": GraphQLID}
}
schema, _ = generate_schema(
self.orientdb_client, # type: ignore # from fixture
self.orientdb_client,
class_to_field_type_overrides=class_to_field_type_overrides,
hidden_classes={ORIENTDB_BASE_VERTEX_CLASS_NAME},
)
Expand Down
Loading

0 comments on commit 8d69f4e

Please sign in to comment.