Unification of Query Answers through common data structures - Part 2 (t…

…ypedb#2913)

# Why is this PR needed?

This PR is a continuation of typedb#2910 

> It has been difficult to maintain and guarantee that various type of Graql Queries (e.g. get, insert, delete, aggregates min/max/sum/groups/etc, compute cluster/path/centrality/statistics and a few others) is always maintained through remote communication APIs (previously REST, now GRPC). Prior to this PR, `AggregateQuery` and `ComputeQuery` are error-prone and not always supported over GRPC. The main reason for this problem is because we have so many different return types for every query, that it becomes hard to guarantee that all queries are supported in a generic and reliable way.

> In order to solve this problem, we need to consider how to unify all queries to return a common set of data structures. By guaranteeing that every query returns a common set of data structures, we can guarantee that every query is supported over the remote communication API (i.e. GRPC) because we have implemented the generic and common data structures.

> Because of the nature of the problem, the solution will be a pretty big change in the code base. Thus, we will split it into multiple PRs and this is the second one.

# What does the PR do?

The next data structures we have identified is:
- `AnswerGroup`: a list of `Answer`s as the members, and a `Concept` as the owner.

The following changes were made in this PR:
- implemented `AnswerGroup<T>`
- unify all `AggregateQuery`s to return `List<Answer>`
- made `ComputeQuery` and `AggregateQuery` implement streamable
- implemented `AnswerGroup` to be sent over GRPC
- removed NULL returns from `Transaction.Query.Iter` in GRPC
- made all queries to have `.stream()` method and removed Streamble<T> interface
- added downcasting methods for `Answer` classes
- moved `ServerRPCIT` to `RemoteQueryIT`, from `engine` to `client` in `test-integration`
- removed `.sub(..)` from `RemoteSchemaConcept`
- made DeleteQuery returns ConceptSet
- made UndefineQuery returns ConceptMap
- made AggregateQuery returns a stream over GRPC
- Cleaned up `QueryExecutor.run(..)` return types to always return `Stream<T extends Answer>`
- CountQuery takes in variables and computes over distinct results
- upgrade `client-nodejs` to support different types of `Answer`s that a Graql query can return
- removed `dist` folder from `client-nodjs`.


# Does it break backwards compatibility?

Nope.

# List of future improvements not on this PR
Yes, plenty, such as (not limited to):

- Make analytics return `Concept` rather than just `ConceptId`
- Add more expressive test for evert Graql query executing over GRPC

README.md

@@ -48,17 +48,9 @@ With the expressivity of the schema, inference through OLTP and distributed algo
 
 - Unix-based Operating Systems (Linux and Mac OSX)
 - Java 8 (OpenJDK or Oracle Java) with the $JAVA_HOME set accordingly
+- Yarn (in order to build Dashboard)
 
-**This repo uses submodules**, so clone using:
-```
-$ git clone --recurse-submodules https://github.com/graknlabs/grakn.git
-```
-Or if you have already cloned:
-```
-$ git submodule update --init --recursive
-```
-
-Then you can build Grakn using Maven:
+You can build Grakn using Maven:
 ```
 $ mvn package -DskipTests
 ```

client-java/src/main/java/ai/grakn/client/Grakn.java

@@ -220,16 +220,12 @@ public void commit() throws InvalidKBException {
         public java.util.Iterator query(Query<?> query) {
             transceiver.send(RequestBuilder.Transaction.query(query.toString(), query.inferring()));
             SessionProto.Transaction.Res txResponse = responseOrThrow();
-
-            switch (txResponse.getQueryIter().getIterCase()) {
-                case NULL:
-                    return Collections.emptyIterator();
-                case ID:
-                    int iteratorId = txResponse.getQueryIter().getId();
-                    return new Iterator<>(this, iteratorId, response -> ResponseReader.answer(response.getQueryIterRes().getAnswer(), this));
-                default:
-                    throw CommonUtil.unreachableStatement("Unexpected " + txResponse);
-            }
+            int iteratorId = txResponse.getQueryIter().getId();
+            return new Iterator<>(
+                    this,
+                    iteratorId,
+                    response -> ResponseReader.answer(response.getQueryIterRes().getAnswer(), this)
+            );
         }
 
         @Nullable

client-java/src/main/java/ai/grakn/client/concept/RemoteSchemaConcept.java

@@ -46,15 +46,6 @@ public final SomeSchemaConcept sup(SomeSchemaConcept type) {
         return asCurrentBaseType(this);
     }
 
-    public final SomeSchemaConcept sub(SomeSchemaConcept type) {
-        ConceptProto.Method.Req method = ConceptProto.Method.Req.newBuilder()
-                .setSchemaConceptSetSupReq(ConceptProto.SchemaConcept.SetSup.Req.newBuilder()
-                        .setSchemaConcept(RequestBuilder.Concept.concept(this))).build();
-
-        runMethod(type.id(), method);
-        return asCurrentBaseType(this);
-    }
-
     @Override
     public final Label label() {
         ConceptProto.Method.Req method = ConceptProto.Method.Req.newBuilder()

client-java/src/main/java/ai/grakn/client/executor/RemoteComputeExecutor.java

@@ -39,7 +39,7 @@ public static <T extends Answer> RemoteComputeExecutor<T> of(Stream<T> result) {
     }
 
     @Override
-    public Stream<T> get() {
+    public Stream<T> stream() {
         return result;
     }
 

client-java/src/main/java/ai/grakn/client/executor/RemoteQueryExecutor.java

@@ -20,6 +20,7 @@
 
 import ai.grakn.ComputeExecutor;
 import ai.grakn.QueryExecutor;
+import ai.grakn.client.Grakn;
 import ai.grakn.graql.AggregateQuery;
 import ai.grakn.graql.ComputeQuery;
 import ai.grakn.graql.DefineQuery;
@@ -30,10 +31,8 @@
 import ai.grakn.graql.UndefineQuery;
 import ai.grakn.graql.answer.Answer;
 import ai.grakn.graql.answer.ConceptMap;
-import ai.grakn.client.Grakn;
-import com.google.common.collect.Iterators;
+import ai.grakn.graql.answer.ConceptSet;
 
-import java.util.Iterator;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
@@ -53,35 +52,35 @@ public static RemoteQueryExecutor create(Grakn.Transaction tx) {
     }
 
     @Override
-    public Stream<ConceptMap> run(GetQuery query) {
-        return streamConceptMaps(query);
+    public Stream<ConceptMap> run(DefineQuery query) {
+        Iterable<ConceptMap> iterable = () -> tx.query(query);
+        return StreamSupport.stream(iterable.spliterator(), false);
     }
 
     @Override
-    public Stream<ConceptMap> run(InsertQuery query) {
+    public Stream<ConceptMap> run(UndefineQuery query) {
         return streamConceptMaps(query);
     }
 
     @Override
-    public void run(DeleteQuery query) {
-        runVoid(query);
+    public Stream<ConceptMap> run(GetQuery query) {
+        return streamConceptMaps(query);
     }
 
     @Override
-    public ConceptMap run(DefineQuery query) {
-        return (ConceptMap) Iterators.getOnlyElement(tx.query(query));
+    public Stream<ConceptMap> run(InsertQuery query) {
+        return streamConceptMaps(query);
     }
 
     @Override
-    public void run(UndefineQuery query) {
-        runVoid(query);
+    public Stream<ConceptSet> run(DeleteQuery query) {
+        return streamConceptSets(query);
     }
 
     @Override
-    public <T> T run(AggregateQuery<T> query) {
-        Iterator iterator = tx.query(query);
-        if (iterator.hasNext()) return (T) Iterators.getOnlyElement(iterator);
-        else return null;
+    public <T extends Answer> Stream<T> run(AggregateQuery<T> query) {
+        Iterable<T> iterable = () -> tx.query(query);
+        return StreamSupport.stream(iterable.spliterator(), false);
     }
 
     @Override
@@ -91,13 +90,15 @@ public <T extends Answer> ComputeExecutor<T> run(ComputeQuery<T> query) {
         return RemoteComputeExecutor.of(stream);
     }
 
-    private void runVoid(Query<?> query) {
-        tx.query(query).forEachRemaining(empty -> {});
+    // Helper methods
+
+    private Stream<ConceptMap> streamConceptMaps(Query<ConceptMap> query) {
+        Iterable<ConceptMap> iterable = () -> tx.query(query);
+        return StreamSupport.stream(iterable.spliterator(), false);
     }
 
-    private Stream<ConceptMap> streamConceptMaps(Query<?> query) {
-        Iterable<Object> iterable = () -> tx.query(query);
-        Stream<Object> stream = StreamSupport.stream(iterable.spliterator(), false);
-        return stream.map(ConceptMap.class::cast);
+    private Stream<ConceptSet> streamConceptSets(Query<ConceptSet> query) {
+        Iterable<ConceptSet> iterable = () -> tx.query(query);
+        return StreamSupport.stream(iterable.spliterator(), false);
     }
 }

client-java/src/main/java/ai/grakn/client/rpc/ResponseReader.java

@@ -24,6 +24,7 @@
 import ai.grakn.graql.Graql;
 import ai.grakn.graql.Var;
 import ai.grakn.graql.answer.Answer;
+import ai.grakn.graql.answer.AnswerGroup;
 import ai.grakn.graql.answer.ConceptList;
 import ai.grakn.graql.answer.ConceptMap;
 import ai.grakn.graql.answer.ConceptSet;
@@ -46,6 +47,8 @@ public class ResponseReader {
 
     public static Answer answer(AnswerProto.Answer res, Grakn.Transaction tx) {
         switch (res.getAnswerCase()) {
+            case ANSWERGROUP:
+                return answerGroup(res.getAnswerGroup(), tx);
             case CONCEPTMAP:
                 return conceptMap(res.getConceptMap(), tx);
             case CONCEPTLIST:
@@ -62,6 +65,12 @@ public static Answer answer(AnswerProto.Answer res, Grakn.Transaction tx) {
         }
     }
 
+    static AnswerGroup<?> answerGroup(AnswerProto.AnswerGroup res, Grakn.Transaction tx) {
+        return new AnswerGroup<>(
+                RemoteConcept.of(res.getOwner(), tx),
+                res.getAnswersList().stream().map(answer -> answer(answer, tx)).collect(toList())
+        );
+    }
     static ConceptMap conceptMap(AnswerProto.ConceptMap res, Grakn.Transaction tx) {
         ImmutableMap.Builder<Var, ai.grakn.concept.Concept> map = ImmutableMap.builder();
         res.getMapMap().forEach((resVar, resConcept) -> {

client-java/src/test/java/ai/grakn/client/TransactionTest.java

@@ -142,23 +142,6 @@ public void whenCreatingAGraknRemoteTxWithSession_SetTxTypeOnTx() {
         }
     }
 
-    @Test
-    public void whenExecutingAQueryWithAVoidResult_GetANullBack() {
-        Query<?> query = match(var("x").isa("person")).delete("x");
-        String queryString = query.toString();
-
-        Transaction.Res response = SessionProto.Transaction.Res.newBuilder()
-                .setQueryIter(SessionProto.Transaction.Query.Iter.newBuilder()
-                        .setNull(ConceptProto.Null.getDefaultInstance())).build();
-
-        server.setResponse(RequestBuilder.Transaction.query(query), response);
-
-        try (Grakn.Transaction tx = session.transaction(GraknTxType.WRITE)) {
-            verify(server.requestListener()).onNext(any()); // The open request
-            assertNull(tx.graql().parse(queryString).execute());
-        }
-    }
-
     @Test(timeout = 5_000)
     public void whenStreamingAQueryWithInfiniteAnswers_Terminate() {
         Transaction.Res queryIterator = SessionProto.Transaction.Res.newBuilder()

client-nodejs/.gitignore

@@ -1,2 +1,3 @@
 node_modules
+dist
 **/autogenerated/*

client-nodejs/README.md

@@ -97,19 +97,73 @@ on every iterator the following methods are available:
 | ------------------------- | --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | async `next()`            | *IteratorElement* or *null* | Retrieves next element or returns null when no more elements are available                                                                                                                                                                                                |
 | async `collect()`         | Array of *IteratorElement*  | Consumes the iterator and collect all the elements into an array                                                                                                                                                                                                          |
-| async `collectConcepts()` | Array of *Concept*          | Consumes the iterator and return array of Concepts. **This helper is only available on Iterator returned by transaction.query() method**. It is useful when one wants to work directly on Concepts without the need to traverse the result map or access the explanation. |
+| async `collectConcepts()` | Array of *Concept*          | Consumes the iterator and return array of Concepts. **This helper is only available on Iterator returned by transaction.query() method when executing a MATCH query.**. It is useful when one wants to work directly on Concepts without the need to traverse the result map or access the explanation. |
 
 **IteratorElement**
 
 Element handled by iterators, depending on the type of iterator this can be a type of *Concept* or an *Answer*.
 
 **Answer**
 
-This object represents a query answer and it is contained in the Iterator returned by `transaction.query()` method, the following methods are available:
+This object represents a query answer and it is contained in the Iterator returned by `transaction.query()` method.     
+There are **different types of Answer**, based on the type of query executed a different type of Answer will be returned:   
+
+| Query Type                           | Answer Type       |
+|--------------------------------------|-------------------|
+| `define`                               | ConceptMap        |
+| `undefine`                             | ConceptMap        |
+| `get`                                  | ConceptMap        |
+| `insert`                               | ConceptMap        |
+| `delete`                               | ConceptMap        |
+| `aggregate count/min/max/sum/mean/std` |  Value            |
+| `aggregate group`                      | AnswerGroup       |
+| `compute count/min/max/sum/mean/std`   | Value             |
+| `compute path`                         | ConceptList       |
+| `compute cluster`                      | ConceptSet        |
+| `compute centrality`                  | ConceptSetMeasure |
+
+**ConceptMap**
 
 | Method          | Return type              | Description                                                                                     |
 | --------------- | ------------------------ | ----------------------------------------------------------------------------------------------- |
-| `get()`         | Map<*String*, *Concept*> | Returns result map in which every variable name (key) is linked to a Concept.                   |
+| `map()`         | Map<*String*, *Concept*> | Returns result map in which every variable name (key) is linked to a Concept.                   |
+| `explanation()` | *Explanation* or *null*  | Returns an Explanation object if the current Answer contains inferred Concepts, null otherwise. |
+
+**Value**
+
+| Method          | Return type              | Description                                                                                     |
+| --------------- | ------------------------ | ----------------------------------------------------------------------------------------------- |
+| `number()`      | Number                   | Returns numeric value of the Answer.                                                            |
+| `explanation()` | *Explanation* or *null*  | Returns an Explanation object if the current Answer contains inferred Concepts, null otherwise. |
+
+**ConceptList**
+
+| Method          | Return type              | Description                                                                                     |
+| --------------- | ------------------------ | ----------------------------------------------------------------------------------------------- |
+| `list()`        | Array of *String*        | Returns list of Concept IDs.                                                                    |
+| `explanation()` | *Explanation* or *null*  | Returns an Explanation object if the current Answer contains inferred Concepts, null otherwise. |
+
+**ConceptSet**
+
+| Method          | Return type              | Description                                                                                     |
+| --------------- | ------------------------ | ----------------------------------------------------------------------------------------------- |
+| `set()`         | Set of *String*          | Returns a set containing Concept IDs.                                                           |
+| `explanation()` | *Explanation* or *null*  | Returns an Explanation object if the current Answer contains inferred Concepts, null otherwise. |
+
+**ConceptSetMeasure**
+
+| Method          | Return type              | Description                                                                                     |
+| --------------- | ------------------------ | ----------------------------------------------------------------------------------------------- |
+| `measurement()` | Number                   | Returns numeric value that is associated to the set of Concepts contained in the current Answer.|
+| `set()`         | Set of *String*          | Returns a set containing Concept IDs.                                                           |
+| `explanation()` | *Explanation* or *null*  | Returns an Explanation object if the current Answer contains inferred Concepts, null otherwise. |
+
+**AnswerGroup**
+
+| Method          | Return type              | Description                                                                                     |
+| --------------- | ------------------------ | ----------------------------------------------------------------------------------------------- |
+| `owner()`       | *Concept*                | Returns the Concepts which is the group owner.                                                  |
+| `answers()`     | Array of *Answer*        | Returns list of Answers that belongs to this group.                                             |
 | `explanation()` | *Explanation* or *null*  | Returns an Explanation object if the current Answer contains inferred Concepts, null otherwise. |
 
 **Explanation**
@@ -120,6 +174,7 @@ This object represents a query answer and it is contained in the Iterator return
 | `answers()`      | Array of *Answer* | Set of deducted/factual answers that allowed us to determine that the owning answer is true |
 
 
+
 **Concepts hierarchy** 
 
 Grakn is composed of different types of Concepts, that have a specific hierarchy
@@ -192,7 +247,7 @@ A `SchemaConcept` concept has all the `Concept` methods plus the following:
  | async `type()`                       | *Type*                     | Returns a Type which is the type of this Thing. This Thing is an instance of that type.                                                                                           |
  | async `relationships(...Role)`       | Iterator of *Relationship* | Returns Relationships which this Thing takes part in, which may **optionally** be narrowed to a particular set according to the Roles you are interested in                       |
  | async `attributes(...AttributeType)` | Iterator of *Attribute*    | Returns Attributes attached to this Thing, which may **optionally** be narrowed to a particular set according to the AttributeTypes you are interested in                         |
- | async `plays()`                      | Iterator of *Role*         | Returns the Roles that this Thing is currently playing                                                                                                                            |
+ | async `roles()`                      | Iterator of *Role*         | Returns the Roles that this Thing is currently playing                                                                                                                            |
  | async `keys(...Attributetype)`       | Iterator of *Attribute*    | Returns a collection of Attribute attached to this Thing as a key, which may **optionally** be narrowed to a particular set according to the AttributeTypes you are interested in |
  | async `has(Attribute)`               | *void*                     | Attaches the provided Attribute to this Thing                                                                                                                                     |
  | async `unhas(Attribute)`             | *void*                     | Removes the provided Attribute from this Thing                                                                                                                                    |