Add id(a) support to front-end #131

andyfengHKU · 2021-04-18T15:52:16Z

This PR add node id comparison support for front-end and should solve issue #102 and issue #127
Example MATCH (a)->(b) WHERE id(a) = id(b) RETURN *

id(a) binds to a logical property expression a._id
Change back-end node value vector name from a to a._id
- Scan(a) -> Scan(a._id)
- Extend(a to b) -> Extend(a._id to b._id)
- ScanNodePR(a.name from a) -> ScanNodeProperty(a.name from a._id)
Rewrite self loop edge (a)->(a) as (a)->(anonymou) WHERE a._id = anonymous._id

andyfengHKU · 2021-04-27T02:50:06Z

src/planner/include/logical_plan/operator/extend/logical_extend.h

@@ -1,45 +1,36 @@
 #pragma once

-#include <string>
+#include <utility>


remove this include

andyfengHKU · 2021-04-27T02:50:54Z

src/planner/include/logical_plan/operator/scan_property/logical_scan_node_property.h

@@ -1,31 +1,30 @@
 #pragma once

-#include <string>
+#include <utility>


andyfengHKU · 2021-04-27T02:51:53Z

src/common/include/types.h

 };

 const string DataTypeNames[] = {
-    "REL", "NODE", "LABEL", "BOOL", "INT32", "INT64", "DOUBLE", "STRING", "UNKNOWN"};
+    "REL", "NODE", "LABEL", "BOOL", "INT32", "INT64", "DOUBLE", "STRING", "NODE_ID", "UNKNOWN"};


"UNSTRUCTURED"

queryproc

Simple PR to understand, just change the terminology used.
Let's add end-to-end tests that use the id() comparison operations as part of this PR.

queryproc · 2021-04-28T05:17:53Z

src/expression/include/logical/logical_rel_expression.h

@@ -15,6 +15,12 @@ class LogicalRelExpression : public LogicalExpression {

    inline string getDstNodeName() const { return dstNode->name; }

+    inline bool isSelfLoopRel() const { return srcNode->name == dstNode->name; }


why is selfLoop part of adding id() comparison support?

because selfLoop edge is rewrite as a regular edge with an ID equal filter (which is also an ID comparison)

queryproc · 2021-04-28T05:18:34Z

src/expression/logical/logical_expression.cpp

@@ -17,9 +17,9 @@ LogicalExpression::LogicalExpression(
 }

 LogicalExpression::LogicalExpression(
-    ExpressionType expressionType, DataType dataType, const string& variableName)
+    ExpressionType expressionType, DataType dataType, const string& propertyOrFunctionName)


The only function name allowed here is id()?

queryproc · 2021-04-28T05:19:03Z

src/planner/binder.cpp

-        if (FUNCTION == expression->expressionType && COUNT_STAR == expression->variableName &&
-            1 != expressions.size()) {
+        if (FUNCTION == expression->expressionType &&
+            FUNCTION_COUNT_STAR == expression->propertyOrFunctionName && 1 != expressions.size()) {


flip the !=
expressions.size() != 1 reads better.

queryproc · 2021-04-28T08:06:36Z

src/planner/enumerator.cpp

@@ -299,18 +307,18 @@ void Enumerator::appendNecessaryScans(shared_ptr<LogicalExpression> expression,
 void Enumerator::appendScanNodeProperty(
    const string& nodeName, const string& propertyName, LogicalPlan& plan) {
    auto queryNode = mergedQueryGraph->getQueryNode(nodeName);
-    auto scanProperty = make_shared<LogicalScanNodeProperty>(


use queryNode->name for consistency.

queryproc · 2021-04-28T08:09:27Z

src/planner/include/logical_plan/operator/extend/logical_extend.h

-    label_t boundNodeVarLabel;
-    string nbrNodeVarName;
-    label_t nbrNodeVarLabel;
+    string boundNodeIDPropertyName;


boundAttributeName where attribute is ID or property works. (otherwise add or in the name).

If you use the term attribute then use it everywhere where you refer to NodeIDPropertyname.

This terminology is used in existing scan and read operators. We have a ScanAttribute operator shared by ScanNodeID, ScanStructuredProperty, and ScanUnstructuredProperty. So you won't be introducing the term attribute.

andyfengHKU requested a review from queryproc April 18, 2021 15:52

andyfengHKU force-pushed the id-expr branch from 1eba57c to 0ec97cf Compare April 27, 2021 02:46

andyfengHKU commented Apr 27, 2021

View reviewed changes

queryproc approved these changes Apr 28, 2021

View reviewed changes

andyfengHKU force-pushed the id-expr branch 2 times, most recently from 070f693 to 1efbfc1 Compare May 2, 2021 01:00

queryproc force-pushed the id-expr branch from 4d4b1f4 to 6e255af Compare May 2, 2021 08:59

add id(a) support to front-end

b4a5d09

queryproc force-pushed the id-expr branch from 6e255af to b4a5d09 Compare May 2, 2021 09:09

queryproc merged commit fd26c25 into master May 2, 2021

queryproc deleted the id-expr branch May 2, 2021 09:23

andyfengHKU mentioned this pull request May 2, 2021

Change the naming of "a" to "a.id" if "a" only represents nodeID vector #102

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add id(a) support to front-end #131

Add id(a) support to front-end #131

andyfengHKU commented Apr 18, 2021 •

edited

andyfengHKU Apr 27, 2021

andyfengHKU Apr 28, 2021

andyfengHKU Apr 27, 2021

andyfengHKU Apr 28, 2021

andyfengHKU Apr 27, 2021

andyfengHKU Apr 28, 2021

queryproc left a comment

queryproc Apr 28, 2021

andyfengHKU Apr 28, 2021

queryproc Apr 28, 2021

queryproc Apr 28, 2021

queryproc Apr 28, 2021

queryproc Apr 28, 2021

		@@ -15,6 +15,12 @@ class LogicalRelExpression : public LogicalExpression {

		inline string getDstNodeName() const { return dstNode->name; }

		inline bool isSelfLoopRel() const { return srcNode->name == dstNode->name; }

Add id(a) support to front-end #131

Add id(a) support to front-end #131

Conversation

andyfengHKU commented Apr 18, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

queryproc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyfengHKU commented Apr 18, 2021 •

edited