Introduce further Query Options (#112)

## What is the goal of this PR?
We introduce modified `infer`, and new `batch_size`, and `explain` options for queries:
```
Transaction.query("...", 
  infer=Transaction.Options.SERVER_DEFAULT, 
  explain=Transaction.Options.SERVER_DEFAULT,   
  batch_size=Transaction.Options.SERVER_DEFAULT)
```

The default `SERVER_DEFAULT` value means that the server will automatically choose the default value for `infer`, `explain`, and `batch_size`. For reference, the server will default to `infer = True`, `explain = True`, and `batch_size = 50`.

** use `explain=true` if you want to retrieve explanations from your query ** This was introduced to ensure correct Explanations, without blowing up Transaction memory when not required.

## What are the changes implemented in this PR?
* Use default value on `transaction.query` such that:
  - `infer=Transaction.Options.SERVER_DEFAULT`
  - `explain=Transaction.Options.SERVER_DEFAULT`
  - `batch_size=Transaction.Options.SERVER_DEFAULT`
* Throw exceptions if the types of parameters are passed in are incorrect.
* Tests for `explain` and `infer` flag

Author: flyingsilverfin

dependencies/graknlabs/dependencies.bzl

@@ -28,12 +28,12 @@ def graknlabs_grakn_core():
     git_repository(
         name = "graknlabs_grakn_core",
         remote = "https://github.com/graknlabs/grakn",
-        commit = "05bcb88d38d1c63f1598699ac4d7e58449fd6726" # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_grakn_core
+        commit = "c3caeb7b626b41f0a0a8784af11840d682d9b17c" # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_grakn_core
     )
 
 def graknlabs_protocol():
     git_repository(
         name = "graknlabs_protocol",
         remote = "https://github.com/graknlabs/protocol",
-        commit = "4ad2dfeaeca85f4ba2f2b42c263d3adf3e36acc3" # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_protocol
+        tag = "1.0.6" # sync-marker: do not remove this comment, this is used for sync-dependencies by @graknlabs_protocol
     )

grakn/client.py

@@ -21,7 +21,7 @@
 
 from grakn.service.Session.util.enums import ValueType   # user-facing ValueType enum
 
-from grakn.service.Session.util.RequestBuilder import RequestBuilder
+from grakn.service.Session.util.RequestBuilder import RequestBuilder, QueryOptions
 from grakn.service.Session.util.enums import TxType as _TxType
 from grakn.service.Keyspace.KeyspaceService import KeyspaceService
 from grakn.service.Session.TransactionService import TransactionService
@@ -132,6 +132,8 @@ def write(self):
 class Transaction(object):
     """ Presents the Grakn interface to the user, actual work with GRPC happens in TransactionService """
 
+    Options = QueryOptions
+
     def __init__(self, transaction_service):
         self._tx_service = transaction_service
     __init__.__annotations__ = {'transaction_service': TransactionService}
@@ -148,9 +150,9 @@ def __exit__(self, type, value, tb):
             #print("Closing Transaction due to exception: {0} \n traceback: \n {1}".format(type, tb))
             return False
 
-    def query(self, query, infer=True):
-        """ Execute a Graql query, inference is optionally enabled """
-        return self._tx_service.query(query, infer)
+    def query(self, query, infer=Options.SERVER_DEFAULT, explain=Options.SERVER_DEFAULT, batch_size=Options.SERVER_DEFAULT):
+        """ Execute a Graql query with query options"""
+        return self._tx_service.query(query, infer, explain, batch_size)
     query.__annotations__ = {'query': str}
 
     def commit(self):

grakn/service/Session/TransactionService.py

@@ -43,9 +43,9 @@ def __init__(self, session_id, tx_type, transaction_endpoint):
     # --- Passthrough targets ---
     # targets of top level Transaction class
 
-    def query(self, query, infer=True, batch_size=50):
+    def query(self, query, infer, explain, batch_size):
         return Iterator(self._communicator,
-                        RequestBuilder.start_iterating_query(query, infer, batch_size),
+                        RequestBuilder.start_iterating_query(query, infer, explain, batch_size),
                         ResponseReader.ResponseReader.get_query_results(self))
     query.__annotations__ = {'query': str}
 

grakn/service/Session/util/RequestBuilder.py

@@ -23,18 +23,24 @@
 import grakn_protocol.session.Answer_pb2 as answer_messages
 from grakn.service.Session.util import enums
 from grakn.service.Session.Concept import BaseTypeMapping
+from grakn.exception import GraknError
 
+class QueryOptions(object):
+    SERVER_DEFAULT = None
+    BATCH_ALL = "all"
 
 class RequestBuilder(object):
     """ Static methods for generating GRPC requests """
 
     @staticmethod
-    def _base_iterate_with_options(batch_size=None):
+    def _base_iterate_with_options(batch_size):
         iter_options = transaction_messages.Transaction.Iter.Req.Options()
-        if batch_size is None:
+        if batch_size == QueryOptions.BATCH_ALL:
             iter_options.all = True
-        else:
+        elif type(batch_size) == int and batch_size > 0:
             iter_options.number = batch_size
+        elif batch_size != QueryOptions.SERVER_DEFAULT:
+            raise GraknError("batch_size parameter must either be an integer, SERVER_DEFAULT, or BATCH_ALL")
 
         transaction_iter_req = transaction_messages.Transaction.Iter.Req()
         transaction_iter_req.options.CopyFrom(iter_options)
@@ -47,12 +53,26 @@ def iter_req_to_tx_req(grpc_iter_req):
         return transaction_req
 
     @staticmethod
-    def start_iterating_query(query, infer=True, batch_size=None):
+    def _query_options(infer, explain):
+        options_message = transaction_messages.Transaction.Query.Options()
+        if infer != QueryOptions.SERVER_DEFAULT:
+            if type(infer) == bool:
+                options_message.inferFlag = infer
+            else:
+                raise GraknError("query 'infer' flag must be SERVER_DEFAULT or a boolean")
+        if explain != QueryOptions.SERVER_DEFAULT:
+            if type(explain) == bool:
+                options_message.explainFlag = explain
+            else:
+                raise GraknError("query 'explain' flag must be SERVER_DEFAULT or a boolean")
+        return options_message
+
+    @staticmethod
+    def start_iterating_query(query, infer, explain, batch_size):
         query_message = transaction_messages.Transaction.Query.Iter.Req()
         query_message.query = query
-        query_message.infer = transaction_messages.Transaction.Query.INFER.Value("TRUE") if infer else \
-            transaction_messages.Transaction.Query.INFER.Value("FALSE")
-
+        query_options = RequestBuilder._query_options(infer, explain)
+        query_message.options.CopyFrom(query_options)
         transaction_iter_req = RequestBuilder._base_iterate_with_options(batch_size)
         transaction_iter_req.query_iter_req.CopyFrom(query_message)
         return transaction_iter_req

tests/integration/test_answer.py

@@ -209,8 +209,10 @@ def test_conceptmap_explanation(self):
             self.assertEqual(has_explanation, 6)
         client.keyspaces().delete("transitivity")
 
-    def test_get_explanation(self):
-        with client.session("explanation") as local_session:
+
+    def test_get_explanation_has_rule(self):
+        """ Test that explanations have rules attached """
+        with client.session("explanation_has_rule") as local_session:
             tx = local_session.transaction().write()
             tx.query("define family-name sub attribute, value string;"
                      "parenthood sub relation, relates parent, relates child;"
@@ -225,14 +227,69 @@ def test_get_explanation(self):
             tx.commit()
             tx = local_session.transaction().read()
 
-            answers = tx.query("match $x isa person, has family-name $f; get;")
+            answers = tx.query("match $x isa person, has family-name $f; get;", explain=True)
             for x in answers:
                 if x.has_explanation():
                     explanation = x.explanation()
                     self.assertIsNotNone(explanation.get_rule())
 
-        client.keyspaces().delete("explanation")
+        client.keyspaces().delete("explanation_has_rule")
+
+
+    def test_query_with_explain_false_no_explanations_available(self):
+        with client.session("query_explain_false") as local_session:
+            tx = local_session.transaction().write()
+            tx.query("define family-name sub attribute, value string;"
+                     "parenthood sub relation, relates parent, relates child;"
+                     "person sub entity, has family-name, plays parent, plays child;"
+                     "family-name-inheritence sub rule,"
+                     "when { (parent: $p, child: $c) isa parenthood; $p has family-name $f; },"
+                     "then { $c has family-name $f; };")
+            tx.query("insert $bob isa person, has family-name \"bobson\";"
+                     "$bobjr isa person;"
+                     "(parent: $bob, child: $bobjr) isa parenthood;")
+
+            tx.commit()
+            tx = local_session.transaction().read()
+
+            answers = tx.query("match $x isa person, has family-name $f; get;", explain=False)
+            for x in answers:
+                self.assertFalse(x.has_explanation())
+
+        client.keyspaces().delete("query_explain_false")
+
+
+    def test_explain_true_explanation_sub_explanation_exists(self):
+        with client.session("query_explain_sub_explanation") as local_session:
+            tx = local_session.transaction().write()
+            tx.query("define family-name sub attribute, value string;"
+                     "parenthood sub relation, relates parent, relates child;"
+                     "fatherhood sub parenthood, relates father as parent, relates father-child as child;"
+                     "person sub entity, has family-name, plays parent, plays child, plays father, plays father-child, has gender;"
+                     "gender sub attribute, value string; "
+                     "family-name-inheritence sub rule,"
+                     "when { (parent: $p, child: $c) isa parenthood; $p has family-name $f; },"
+                     "then { $c has family-name $f; }; "
+                     "fatherhood-rule sub rule, "
+                     "when { $p isa person, has gender \"male\", has family-name $f; $c isa person, has family-name $f; }, "
+                     "then { (father: $p, father-child: $c) isa fatherhood; };")
+            tx.query("insert $bob isa person, has family-name \"bobson\", has gender \"male\";"
+                     "$bobjr isa person;"
+                     "(parent: $bob, child: $bobjr) isa parenthood;")
+
+            tx.commit()
+            tx = local_session.transaction().read()
+
+            answers = tx.query("match $m isa fatherhood; get;", explain=True)
+            for x in answers:
+                if x.has_explanation():
+                    explanation = x.explanation()
+                    self.assertIsNotNone(explanation.get_rule())
+                    sub_answers = explanation.get_answers()
+                    self.assertTrue(sub_answers[0].has_explanation())
+                    self.assertIsNotNone(sub_answers[0].explanation())
 
+        client.keyspaces().delete("query_explain_sub_explanation")
 
 if __name__ == "__main__":
     with GraknServer():

tests/integration/test_grakn.py

@@ -20,7 +20,7 @@
 import unittest
 import uuid
 import grakn
-from grakn.client import GraknClient, ValueType
+from grakn.client import GraknClient, ValueType, Transaction
 from grakn.exception.GraknError import GraknError
 from grakn.service.Session.util.ResponseReader import Value
 
@@ -204,6 +204,84 @@ def test_query_invalid_syntax(self):
         self.assertFalse(self.tx.is_open(), msg="Tx is not closed after invalid syntax")
 
 
+    def test_query_infer_false(self):
+        """ Test that when the infer flag is false, no inferred answers are returned """
+        local_session = client.session("query_flag_infer_false")
+        local_tx = local_session.transaction().write()
+        local_tx.query("define person sub entity, has name; name sub attribute, value string; naming sub rule, when {$x isa person; $a isa name;}, then {$x has name $a;};").get()
+        local_tx.query("insert $x isa person; $a \"John\" isa name;").get()
+        local_tx.commit()
+        local_tx = local_session.transaction().read()
+        answers = list(local_tx.query("match $x isa person, has name $a; get;", infer=False).get())
+        self.assertEquals(len(answers), 0)
+
+
+    def test_query_infer_true(self):
+        """ Test that when the infer flag is true, inferred answers are returned """
+        local_session = client.session("query_flag_infer_true")
+        local_tx = local_session.transaction().write()
+        local_tx.query("define person sub entity, has name; name sub attribute, value string; naming sub rule, when {$x isa person; $a isa name;}, then {$x has name $a;};").get()
+        local_tx.query("insert $x isa person; $a \"John\" isa name;").get()
+        local_tx.commit()
+        local_tx = local_session.transaction().read()
+        answers = list(local_tx.query("match $x isa person, has name $a; get;", infer=True).get())
+        self.assertEquals(len(answers), 1)
+
+
+    def test_query_batch_all(self):
+        """ Test that batch_size=ALL succeeds at retrieving all the answers """
+        local_session = client.session("query_batch_size_all")
+        local_tx = local_session.transaction().write()
+        local_tx.query("define person sub entity, has name; name sub attribute, value string;")
+        for i in range(100):
+            local_tx.query("insert $x isa person, has name \"John-{0}\"; ".format(i))
+        local_tx.commit()
+        local_tx = local_session.transaction().read()
+        answers = list(local_tx.query("match $x isa person, has name $a; get;", batch_size=Transaction.Options.BATCH_ALL).get())
+        self.assertEquals(len(answers), 100)
+
+
+    def test_query_batch_one(self):
+        """ Test that batch_size=1 succeeds at retrieving all the answers """
+        local_session = client.session("query_batch_size_one")
+        local_tx = local_session.transaction().write()
+        local_tx.query("define person sub entity, has name; name sub attribute, value string;")
+        for i in range(100):
+            local_tx.query("insert $x isa person, has name \"John-{0}\"; ".format(i))
+        local_tx.commit()
+        local_tx = local_session.transaction().read()
+        answers = list(local_tx.query("match $x isa person, has name $a; get;", batch_size=1).get())
+        self.assertEquals(len(answers), 100)
+
+
+    def test_query_batch_nonpositive_throws(self):
+        """ Test that batch_size="Nonsense" succeeds at retrieving all the answers """
+        local_session = client.session("query_batch_size_nonpositive")
+        local_tx = local_session.transaction().write()
+        local_tx.query("define person sub entity, has name; name sub attribute, value string;")
+        for i in range(100):
+            local_tx.query("insert $x isa person, has name \"John-{0}\"; ".format(i))
+        local_tx.commit()
+        local_tx = local_session.transaction().read()
+        with self.assertRaises(Exception):
+            answers = list(local_tx.query("match $x isa person, has name $a; get;", batch_size=0).get())
+        with self.assertRaises(Exception):
+            answers = list(local_tx.query("match $x isa person, has name $a; get;", batch_size=-50).get())
+
+
+    def test_query_batch_nonsense_throws(self):
+        """ Test that batch_size="Nonsense" succeeds at retrieving all the answers """
+        local_session = client.session("query_batch_size_nonsense")
+        local_tx = local_session.transaction().write()
+        local_tx.query("define person sub entity, has name; name sub attribute, value string;")
+        for i in range(100):
+            local_tx.query("insert $x isa person, has name \"John-{0}\"; ".format(i))
+        local_tx.commit()
+        local_tx = local_session.transaction().read()
+        with self.assertRaises(Exception):
+            answers = list(local_tx.query("match $x isa person, has name $a; get;", batch_size="nonsense").get())
+
+
     def test_query_tx_already_closed(self):
         self.tx.close()
         with self.assertRaises(GraknError):