Skip to content

Query execution in mage#1773

Merged
Josipmrden merged 51 commits intomasterfrom
query-execution-in-mage
Apr 5, 2024
Merged

Query execution in mage#1773
Josipmrden merged 51 commits intomasterfrom
query-execution-in-mage

Conversation

@Josipmrden
Copy link
Contributor

@Josipmrden Josipmrden commented Feb 29, 2024

Query execution in query modules.

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

To keep docs changelog up to date, one more thing to do:

  • Write a release note here, including added/changed clauses "Extended the C API to support query execution. Extension also provided on the C++ level"
  • Tag someone from docs team in the comments @kgolubic

Documentation link: memgraph/documentation#658

@gitbuda gitbuda added this to the mg-v2.16.0 milestone Mar 18, 2024
@Josipmrden Josipmrden requested review from gvolfing and removed request for antoniofilipovic March 28, 2024 16:55
@Josipmrden
Copy link
Contributor Author

Josipmrden commented Mar 29, 2024

Things that I'm not sure yet and looking for pointers @gvolfing @Ignition :

  • How to best make the singleton out of the interpreter context? -> i set it to raw pointer for now, I still have to make the instance constructor private because it's a singleton, need thoughts on this -> UPDATE: Set it to unique pointer so cleanup is ensured, perhaps everything is okay now, still need to make the constructor private

  • Can it be a singleton, if we have MT? UPDATE: Added support for multi-tenancy, but database name needs to be kept in the interpreter and updated

  • Lifetime of pointers in the API -> pls check if I did this correctly

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@Josipmrden Josipmrden removed the draft label Apr 5, 2024
@Josipmrden Josipmrden merged commit 2e6bdcc into master Apr 5, 2024
@Josipmrden Josipmrden deleted the query-execution-in-mage branch April 5, 2024 12:29
@kgolubic
Copy link
Contributor

kgolubic commented Apr 7, 2024

@Josipmrden is there a related docs PR?

@antejavor
Copy link
Contributor

antejavor commented Apr 9, 2024

API works 🎊

Just small observation:

/hello_query_module/hello_query_module.cpp:28:45: error: call to member function 'ExecuteQuery' is ambiguous
        mgp::QueryExecution(memgraph_graph).ExecuteQuery("MATCH n RETURN n;");
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~

Non-casted string leads to ambiguity by the compiler due to multiple versions:

class QueryExecution {
 public:
  QueryExecution(mgp_graph *graph);
  ExecutionResult ExecuteQuery(std::string_view query, Map params = Map()) const;
  ExecutionResult ExecuteQuery(std::string query, Map params = Map()) const;
    auto res = mgp::QueryExecution(memgraph_graph)
                   .ExecuteQuery(std::string("MATCH n RETURN n;"));

Do you know if the two versions are necessary? @Josipmrden

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docs needed Docs needed feature feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Figure out how to safely call memgraph via client from query module (auth concern)

6 participants