ArangoDB-Community / arangodb-tinkerpop-provider

An implementation of the Tinkerpop OLTP Provider for ArangoDB
Apache License 2.0
84 stars 16 forks source link

Why ArangoDBQueryBuilder.documentsById() has 'docs' collection hardcoded? #41

Closed fdominik closed 5 years ago

fdominik commented 5 years ago

When implementing the feature #40 I found, that there is a hard coded name of collection in ArangoDBQueryBuilder (Line 246):

public ArangoDBQueryBuilder documentsById(
        List<String> ids,
        String loopVariable,
        Map<String, Object> bindVars) {
        queryBuilder.append("LET docs = FLATTEN(RETURN Document(@ids))\n");
        queryBuilder.append(String.format("FOR %s IN docs\n", loopVariable));
        queryBuilder.append(String.format("  FILTER NOT IS_NULL(%s)\n", loopVariable)); // Not needed?
        bindVars.put("ids", ids);
        logger.debug("documentsById", queryBuilder.toString());
        return this;
    }

I don't think this should be there, especially when collection Names are created using getCollectionName method from ArangoDBUtils. Also this kind of collection can easily conflict with existing collections (e.g. we use the same name in our project for different purpose).

However I am not that sure of purpose of this method (eventhough the javadoc is present), so I didn't have courage to refactor it :)

arcanefoam commented 5 years ago

Hi, Here 'docs' refers to the AQL variable docsdefined in the previous line; LET docs = .... This methods does a DB wide search for documents matching a list of ids. So this is not related to #40.

For future reference, this method is used for retrieving all edges/nodes (see getGraphEdges and getGraphVertices in ArangoDBGraphClient).