From ebf4222dd7fc3b6a9d1cbe7c973447665da5f0cc Mon Sep 17 00:00:00 2001 From: Predrag Gruevski Date: Sun, 23 Dec 2018 21:51:17 -0500 Subject: [PATCH 1/3] Add reproducing testcase for TypeError on None sub-expression. --- graphql_compiler/tests/test_compiler.py | 12 +++++++++ graphql_compiler/tests/test_input_data.py | 32 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/graphql_compiler/tests/test_compiler.py b/graphql_compiler/tests/test_compiler.py index fb20324f8..7f5efe845 100644 --- a/graphql_compiler/tests/test_compiler.py +++ b/graphql_compiler/tests/test_compiler.py @@ -2203,6 +2203,18 @@ def test_has_edge_degree_op_filter_with_optional(self): check_test_data(self, test_data, expected_match, expected_gremlin) + def test_has_edge_degree_op_filter_with_optional_and_other_filter(self): + test_data = test_input_data.has_edge_degree_op_filter_with_optional_and_other_filter() + + expected_match = ''' + + ''' + expected_gremlin = ''' + + ''' + + check_test_data(self, test_data, expected_match, expected_gremlin) + def test_has_edge_degree_op_filter_with_fold(self): test_data = test_input_data.has_edge_degree_op_filter_with_fold() diff --git a/graphql_compiler/tests/test_input_data.py b/graphql_compiler/tests/test_input_data.py index 535fa62de..2e3592802 100644 --- a/graphql_compiler/tests/test_input_data.py +++ b/graphql_compiler/tests/test_input_data.py @@ -1253,6 +1253,38 @@ def has_edge_degree_op_filter_with_optional(): type_equivalence_hints=None) +def has_edge_degree_op_filter_with_optional_and_other_filter(): + graphql_input = '''{ + Animal { + name @output(out_name: "animal_name") + uuid @filter(op_name: "between", value: ["$uuid_lower_bound","$uuid_upper_bound"]) + + in_Animal_ParentOf @optional + @filter(op_name: "has_edge_degree", value: ["$number_of_edges"]) { + out_Entity_Related { + ... on Event { + name @output(out_name: "related_event") + } + } + } + } + } + ''' + expected_output_metadata = { + 'animal_name': OutputMetadata(type=GraphQLString, optional=False), + 'related_event': OutputMetadata(type=GraphQLString, optional=True), + } + expected_input_metadata = { + 'number_of_edges': GraphQLInt, + } + + return CommonTestData( + graphql_input=graphql_input, + expected_output_metadata=expected_output_metadata, + expected_input_metadata=expected_input_metadata, + type_equivalence_hints=None) + + def has_edge_degree_op_filter_with_fold(): graphql_input = '''{ Species { From ae7c8b237a86b7b16a90c2757045e59ba24c1bb1 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski Date: Tue, 25 Dec 2018 14:42:53 -0500 Subject: [PATCH 2/3] Update input data, add test for correct IR generation. --- graphql_compiler/tests/test_input_data.py | 2 + graphql_compiler/tests/test_ir_generation.py | 89 ++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/graphql_compiler/tests/test_input_data.py b/graphql_compiler/tests/test_input_data.py index 2e3592802..d51f62285 100644 --- a/graphql_compiler/tests/test_input_data.py +++ b/graphql_compiler/tests/test_input_data.py @@ -1275,6 +1275,8 @@ def has_edge_degree_op_filter_with_optional_and_other_filter(): 'related_event': OutputMetadata(type=GraphQLString, optional=True), } expected_input_metadata = { + 'uuid_lower_bound': GraphQLID, + 'uuid_upper_bound': GraphQLID, 'number_of_edges': GraphQLInt, } diff --git a/graphql_compiler/tests/test_ir_generation.py b/graphql_compiler/tests/test_ir_generation.py index 8b0fd38c3..b1db1d83e 100644 --- a/graphql_compiler/tests/test_ir_generation.py +++ b/graphql_compiler/tests/test_ir_generation.py @@ -2238,6 +2238,95 @@ def test_has_edge_degree_op_filter_with_optional(self): check_test_data(self, test_data, expected_blocks, expected_location_types) + def test_has_edge_degree_op_filter_with_optional_and_other_filter(self): + test_data = test_input_data.has_edge_degree_op_filter_with_optional_and_other_filter() + + base_location = helpers.Location(('Animal',)) + parent_location = base_location.navigate_to_subpath('in_Animal_ParentOf') + related_location = parent_location.navigate_to_subpath('out_Entity_Related') + revisited_base_location = base_location.revisit() + + expected_blocks = [ + blocks.QueryRoot({'Animal'}), + blocks.Filter( + expressions.BinaryComposition( + u'||', + expressions.BinaryComposition( + u'&&', + expressions.BinaryComposition( + u'=', + expressions.Variable('$number_of_edges', GraphQLInt), + expressions.ZeroLiteral + ), + expressions.BinaryComposition( + u'=', + expressions.LocalField('in_Animal_ParentOf'), + expressions.NullLiteral + ) + ), + expressions.BinaryComposition( + u'&&', + expressions.BinaryComposition( + u'!=', + expressions.LocalField('in_Animal_ParentOf'), + expressions.NullLiteral + ), + expressions.BinaryComposition( + u'=', + expressions.UnaryTransformation( + u'size', + expressions.LocalField('in_Animal_ParentOf') + ), + expressions.Variable('$number_of_edges', GraphQLInt) + ) + ) + ) + ), + blocks.Filter( + expressions.BinaryComposition( + u'&&', + expressions.BinaryComposition( + u'>=', + expressions.LocalField('uuid'), + expressions.Variable('$uuid_lower_bound', GraphQLID) + ), + expressions.BinaryComposition( + u'<=', + expressions.LocalField('uuid'), + expressions.Variable('$uuid_upper_bound', GraphQLID) + ) + ) + ), + blocks.MarkLocation(base_location), + blocks.Traverse('in', 'Animal_ParentOf', optional=True), + blocks.MarkLocation(parent_location), + blocks.Traverse('out', 'Entity_Related', within_optional_scope=True), + blocks.CoerceType({'Event'}), + blocks.MarkLocation(related_location), + blocks.Backtrack(parent_location), + blocks.EndOptional(), + blocks.Backtrack(base_location, optional=True), + blocks.MarkLocation(revisited_base_location), + blocks.ConstructResult({ + 'animal_name': expressions.OutputContextField( + base_location.navigate_to_field('name'), GraphQLString), + 'related_event': expressions.TernaryConditional( + expressions.ContextFieldExistence(related_location), + expressions.OutputContextField( + related_location.navigate_to_field('name'), GraphQLString), + expressions.NullLiteral), + }), + ] + expected_location_types = { + base_location: 'Animal', + parent_location: 'Animal', + related_location: 'Event', + revisited_base_location: 'Animal', + } + + check_test_data(self, test_data, expected_blocks, expected_location_types) + + def test_has_edge_degree_op_filter_with_fold(self): test_data = test_input_data.has_edge_degree_op_filter_with_fold() From 49423bc2940d2239c1e2a52154f8c7969b0e2174 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski Date: Tue, 25 Dec 2018 15:15:02 -0500 Subject: [PATCH 3/3] Fix lowering error due to missing return statement. --- graphql_compiler/compiler/expressions.py | 4 +- .../compiler/ir_lowering_match/__init__.py | 3 +- .../ir_lowering_match/optional_traversal.py | 7 +- graphql_compiler/tests/test_compiler.py | 96 ++++++++++++++++++- graphql_compiler/tests/test_input_data.py | 2 +- graphql_compiler/tests/test_ir_generation.py | 4 +- 6 files changed, 102 insertions(+), 14 deletions(-) diff --git a/graphql_compiler/compiler/expressions.py b/graphql_compiler/compiler/expressions.py index 11a763bb2..e1cd2b068 100644 --- a/graphql_compiler/compiler/expressions.py +++ b/graphql_compiler/compiler/expressions.py @@ -664,8 +664,8 @@ def validate(self): _validate_operator_name(self.operator, BinaryComposition.SUPPORTED_OPERATORS) if not isinstance(self.left, Expression): - raise TypeError(u'Expected Expression left, got: {} {}'.format( - type(self.left).__name__, self.left)) + raise TypeError(u'Expected Expression left, got: {} {} {}'.format( + type(self.left).__name__, self.left, self)) if not isinstance(self.right, Expression): raise TypeError(u'Expected Expression right, got: {} {}'.format( diff --git a/graphql_compiler/compiler/ir_lowering_match/__init__.py b/graphql_compiler/compiler/ir_lowering_match/__init__.py index 0ecb4b8d3..5812908f9 100644 --- a/graphql_compiler/compiler/ir_lowering_match/__init__.py +++ b/graphql_compiler/compiler/ir_lowering_match/__init__.py @@ -117,8 +117,7 @@ def lower_ir(ir_blocks, query_metadata_table, type_equivalence_hints=None): match_query, complex_optional_roots, location_to_optional_roots) compound_match_query = prune_non_existent_outputs(compound_match_query) compound_match_query = collect_filters_to_first_location_occurrence(compound_match_query) - compound_match_query = lower_context_field_expressions( - compound_match_query) + compound_match_query = lower_context_field_expressions(compound_match_query) compound_match_query = truncate_repeated_single_step_traversals_in_sub_queries( compound_match_query) diff --git a/graphql_compiler/compiler/ir_lowering_match/optional_traversal.py b/graphql_compiler/compiler/ir_lowering_match/optional_traversal.py index 4c1a989cb..985ee5541 100644 --- a/graphql_compiler/compiler/ir_lowering_match/optional_traversal.py +++ b/graphql_compiler/compiler/ir_lowering_match/optional_traversal.py @@ -497,14 +497,15 @@ def _update_context_field_expression(present_locations, expression): if isinstance(lower_bound, ContextField) or isinstance(upper_bound, ContextField): raise AssertionError(u'Found BetweenClause with ContextFields as lower/upper bounds. ' u'This should never happen: {}'.format(expression)) + return expression elif isinstance(expression, (OutputContextField, FoldedOutputContextField)): raise AssertionError(u'Found unexpected expression of type {}. This should never happen: ' u'{}'.format(type(expression).__name__, expression)) elif isinstance(expression, no_op_blocks): return expression - else: - raise AssertionError(u'Found unexpected expression of type {}. This should never happen: ' - u'{}'.format(type(expression).__name__, expression)) + + raise AssertionError(u'Found unhandled expression of type {}. This should never happen: ' + u'{}'.format(type(expression).__name__, expression)) def _lower_non_existent_context_field_filters(match_traversals, visitor_fn): diff --git a/graphql_compiler/tests/test_compiler.py b/graphql_compiler/tests/test_compiler.py index 7f5efe845..ee163ba67 100644 --- a/graphql_compiler/tests/test_compiler.py +++ b/graphql_compiler/tests/test_compiler.py @@ -2203,14 +2203,102 @@ def test_has_edge_degree_op_filter_with_optional(self): check_test_data(self, test_data, expected_match, expected_gremlin) - def test_has_edge_degree_op_filter_with_optional_and_other_filter(self): - test_data = test_input_data.has_edge_degree_op_filter_with_optional_and_other_filter() + def test_has_edge_degree_op_filter_with_optional_and_between(self): + test_data = test_input_data.has_edge_degree_op_filter_with_optional_and_between() expected_match = ''' - + SELECT EXPAND($result) + LET + $optional__0 = ( + SELECT + Animal___1.name AS `animal_name` + FROM ( + MATCH {{ + class: Animal, + where: (( + ( + (uuid BETWEEN {uuid_lower_bound} AND {uuid_upper_bound}) AND + ( + ( + ({number_of_edges} = 0) AND + (in_Animal_ParentOf IS null) + ) OR ( + (in_Animal_ParentOf IS NOT null) AND + (in_Animal_ParentOf.size() = {number_of_edges}) + ) + ) + ) AND ( + (in_Animal_ParentOf IS null) OR + (in_Animal_ParentOf.size() = 0) + ) + )), + as: Animal___1 + }} + RETURN $matches + ) + ), + $optional__1 = ( + SELECT + Animal___1.name AS `animal_name`, + Animal__in_Animal_ParentOf__out_Entity_Related___1.name AS `related_event` + FROM ( + MATCH {{ + class: Animal, + where: (( + (uuid BETWEEN {uuid_lower_bound} AND {uuid_upper_bound}) AND + ( + ( + ({number_of_edges} = 0) AND + (in_Animal_ParentOf IS null) + ) OR ( + (in_Animal_ParentOf IS NOT null) AND + (in_Animal_ParentOf.size() = {number_of_edges}) + ) + ) + )), + as: Animal___1 + }}.in('Animal_ParentOf') {{ + as: Animal__in_Animal_ParentOf___1 + }}.out('Entity_Related') {{ + where: ((@this INSTANCEOF 'Event')), + as: Animal__in_Animal_ParentOf__out_Entity_Related___1 + }} RETURN $matches + ) + ), + $result = UNIONALL($optional__0, $optional__1) ''' expected_gremlin = ''' - + g.V('@class', 'Animal') + .filter{it, m -> ( + ( + ( + ($number_of_edges == 0) && + (it.in_Animal_ParentOf == null) + ) || ( + (it.in_Animal_ParentOf != null) && + (it.in_Animal_ParentOf.count() == $number_of_edges) + ) + ) && ( + (it.uuid >= $uuid_lower_bound) && (it.uuid <= $uuid_upper_bound) + ) + )} + .as('Animal___1') + .ifThenElse{it.in_Animal_ParentOf == null}{null}{it.in('Animal_ParentOf')} + .as('Animal__in_Animal_ParentOf___1') + .ifThenElse{it == null}{null}{it.out('Entity_Related')} + .filter{it, m -> ((it == null) || ['Event'].contains(it['@class']))} + .as('Animal__in_Animal_ParentOf__out_Entity_Related___1') + .back('Animal__in_Animal_ParentOf___1') + .optional('Animal___1') + .as('Animal___2') + .transform{it, m -> new com.orientechnologies.orient.core.record.impl.ODocument([ + animal_name: m.Animal___1.name, + related_event: ( + (m.Animal__in_Animal_ParentOf__out_Entity_Related___1 != null) ? + m.Animal__in_Animal_ParentOf__out_Entity_Related___1.name : + null + ) + ])} ''' check_test_data(self, test_data, expected_match, expected_gremlin) diff --git a/graphql_compiler/tests/test_input_data.py b/graphql_compiler/tests/test_input_data.py index d51f62285..a3b60ad09 100644 --- a/graphql_compiler/tests/test_input_data.py +++ b/graphql_compiler/tests/test_input_data.py @@ -1253,7 +1253,7 @@ def has_edge_degree_op_filter_with_optional(): type_equivalence_hints=None) -def has_edge_degree_op_filter_with_optional_and_other_filter(): +def has_edge_degree_op_filter_with_optional_and_between(): graphql_input = '''{ Animal { name @output(out_name: "animal_name") diff --git a/graphql_compiler/tests/test_ir_generation.py b/graphql_compiler/tests/test_ir_generation.py index b1db1d83e..4aab0731f 100644 --- a/graphql_compiler/tests/test_ir_generation.py +++ b/graphql_compiler/tests/test_ir_generation.py @@ -2238,8 +2238,8 @@ def test_has_edge_degree_op_filter_with_optional(self): check_test_data(self, test_data, expected_blocks, expected_location_types) - def test_has_edge_degree_op_filter_with_optional_and_other_filter(self): - test_data = test_input_data.has_edge_degree_op_filter_with_optional_and_other_filter() + def test_has_edge_degree_op_filter_with_optional_and_between(self): + test_data = test_input_data.has_edge_degree_op_filter_with_optional_and_between() base_location = helpers.Location(('Animal',)) parent_location = base_location.navigate_to_subpath('in_Animal_ParentOf')