Skip to content

Commit

Permalink
fix: Includes necessary JOINs of vertices (#599)
Browse files Browse the repository at this point in the history 
Previously, a patch(3ced882) was created to
remove the items that A thought were unnecessary joins. However, this returned
an exceptional case for the self-joining case without path(other words, Unnamed
path).

so, we rollback that partial of patch. and, we will refactor pattern match
logic for performance issue.
  • Loading branch information
@emotionbug
emotionbug committed Dec 15, 2022
1 parent b37fa7a commit c05c235
Show file tree
Hide file tree
Showing 4 changed files with 211 additions and 117 deletions.
146 changes: 48 additions & 98 deletions src/backend/parser/parse_graph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ static bool arePathsConnected(CypherPath *path1, CypherPath *path2);
static Node *transformComponents(ParseState *pstate, List *components,
List **targetList);
static Node *transformMatchNode(ParseState *pstate, CypherNode *cnode,
bool force, List **targetList, List **eqoList,
List **targetList, List **eqoList,
bool *isNSItem);
static ParseNamespaceItem *transformMatchRel(ParseState *pstate,
CypherRel *crel,
Expand Down Expand Up @@ -1638,14 +1638,12 @@ transformComponents(ParseState *pstate, List *components, List **targetList)
/* `cnode` is the first node in the path */
if (prev_crel == NULL)
{
bool zero;

le = lnext(p->chain, le);

/* vertex only path */
if (le == NULL)
{
vertex = transformMatchNode(pstate, cnode, true,
vertex = transformMatchNode(pstate, cnode,
targetList, &eqoList,
&vertex_is_nsitem);
break;
Expand All @@ -1657,9 +1655,7 @@ transformComponents(ParseState *pstate, List *components, List **targetList)
* if `crel` is zero-length VLE, get RTE of `cnode`
* because `crel` needs `id` column of the RTE
*/
zero = isZeroLengthVLE(crel);
vertex = transformMatchNode(pstate, cnode,
(zero || out), targetList,
vertex = transformMatchNode(pstate, cnode, targetList,
&eqoList, &vertex_is_nsitem);

if (p->kind != CPATH_NORMAL)
Expand All @@ -1679,7 +1675,7 @@ transformComponents(ParseState *pstate, List *components, List **targetList)
}
else
{
vertex = transformMatchNode(pstate, cnode, out, targetList,
vertex = transformMatchNode(pstate, cnode, targetList,
&eqoList, &vertex_is_nsitem);
qual = addQualNodeIn(pstate, qual, vertex,
vertex_is_nsitem, prev_crel,
Expand Down Expand Up @@ -1798,8 +1794,8 @@ transformComponents(ParseState *pstate, List *components, List **targetList)
}

static Node *
transformMatchNode(ParseState *pstate, CypherNode *cnode, bool force,
List **targetList, List **eqoList, bool *isNSItem)
transformMatchNode(ParseState *pstate, CypherNode *cnode, List **targetList,
List **eqoList, bool *isNSItem)
{
char *varname = getCypherName(cnode->variable);
int varloc = getCypherNameLoc(cnode->variable);
Expand All @@ -1808,10 +1804,9 @@ transformMatchNode(ParseState *pstate, CypherNode *cnode, bool force,
char *labname;
int labloc;
bool prop_constr;
Const *id;
Const *prop_map;
Const *tid;
Node *vertex;
RangeVar *r;
Alias *alias;
ParseNamespaceItem *nsitem;

*isNSItem = false;

Expand All @@ -1825,8 +1820,6 @@ transformMatchNode(ParseState *pstate, CypherNode *cnode, bool force,
te = findTarget(*targetList, varname);
if (te != NULL)
{
ParseNamespaceItem *nsitem;

if (exprType((Node *) te->expr) != VERTEXOID)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_ALIAS),
Expand Down Expand Up @@ -1923,98 +1916,55 @@ transformMatchNode(ParseState *pstate, CypherNode *cnode, bool force,
else
vertexLabelExist(pstate, labname, labloc);

/*
* If `cnode` has a label constraint or a property constraint, return RTE.
*
* If `cnode` is in a path, return RTE because the path must consist of
* valid vertices.
*
* If there is no previous relationship of `cnode` in the path and the
* next relationship of `cnode` is zero-length, return RTE because the
* relationship needs starting point.
*/
if (strcmp(labname, AG_VERTEX) != 0 || prop_constr || force)
{
RangeVar *r;
Alias *alias;
ParseNamespaceItem *nsitem;

r = makeRangeVar(get_graph_path(true),
labname,
labloc);
r->inh = !cnode->only;
alias = makeAliasOptUnique(varname);
r = makeRangeVar(get_graph_path(true),
labname,
labloc);
r->inh = !cnode->only;
alias = makeAliasOptUnique(varname);

/* set `ihn` to true because we should scan all derived tables */
nsitem = addRangeTableEntry(pstate, r, alias, r->inh, true);
addNSItemToJoinlist(pstate, nsitem, false);
/* set `ihn` to true because we should scan all derived tables */
nsitem = addRangeTableEntry(pstate, r, alias, r->inh, true);
addNSItemToJoinlist(pstate, nsitem, false);

if (varname != NULL || prop_constr)
{
bool resjunk;
int resno;
if (varname != NULL || prop_constr)
{
bool resjunk;
int resno;

/*
* If `varname` is NULL, this target has to be ignored when
* `RETURN *`.
*/
resjunk = (varname == NULL);
resno = (resjunk ? InvalidAttrNumber : pstate->p_next_resno++);
/*
* If `varname` is NULL, this target has to be ignored when
* `RETURN *`.
*/
resjunk = (varname == NULL);
resno = (resjunk ? InvalidAttrNumber : pstate->p_next_resno++);

te = makeTargetEntry((Expr *) makeVertexExpr(pstate,
nsitem,
varloc),
(AttrNumber) resno,
alias->aliasname,
resjunk);
te = makeTargetEntry((Expr *) makeVertexExpr(pstate,
nsitem,
varloc),
(AttrNumber) resno,
alias->aliasname,
resjunk);

if (resjunk)
{
ElemQualOnly *eqo;
if (resjunk)
{
ElemQualOnly *eqo;

eqo = palloc(sizeof(*eqo));
eqo->te = te;
eqo->prop_map = cnode->prop_map;
eqo = palloc(sizeof(*eqo));
eqo->te = te;
eqo->prop_map = cnode->prop_map;

*eqoList = lappend(*eqoList, eqo);
}
else
{
addElemQual(pstate, te->resno, cnode->prop_map);
*targetList = lappend(*targetList, te);
}
*eqoList = lappend(*eqoList, eqo);
}
else
{
addElemQual(pstate, te->resno, cnode->prop_map);
*targetList = lappend(*targetList, te);
}

/* return RTE to help the caller can access columns directly */
*isNSItem = true;
return (Node *) nsitem;
}

/* this node is just a placeholder for relationships */
if (varname == NULL)
return NULL;

/*
* `cnode` is assigned to the variable `varname` but there is a chance to
* omit the RTE for `cnode` if no expression uses properties of `cnode`.
* So, return a (invalid) future vertex at here for later use.
*/

id = makeNullConst(GRAPHIDOID, -1, InvalidOid);
prop_map = makeNullConst(JSONBOID, -1, InvalidOid);
tid = makeNullConst(TIDOID, -1, InvalidOid);
vertex = makeTypedRowExpr(list_make3(id, prop_map, tid), VERTEXOID, varloc);
te = makeTargetEntry((Expr *) vertex,
(AttrNumber) pstate->p_next_resno++,
varname,
false);

/* there is no need to addElemQual() here */
*targetList = lappend(*targetList, te);

addFutureVertex(pstate, te->resno, labname);

return (Node *) te;
/* return RTE to help the caller can access columns directly */
*isNSItem = true;
return (Node *) nsitem;
}

static ParseNamespaceItem *
Expand Down
38 changes: 19 additions & 19 deletions src/test/regress/expected/cypher_dml.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,8 @@ MATCH (A)-[r:el1 *1..3]->(B) RETURN A.id, r, B.id;
id | r | id
----+--------------------------------------------+----
1 | [el1[9.1][6.1,7.1]{}] | 2
1 | [el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{}] | 3
1 | [el2[10.1][6.1,8.1]{}] | 3
1 | [el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{}] | 3
2 | [el3[11.1][7.1,8.1]{}] | 3
(4 rows)

Expand Down Expand Up @@ -626,8 +626,8 @@ MATCH (A)<-[r:el1 *1..3]-(B) RETURN A.id, r, B.id;
id | r | id
----+--------------------------------------------+----
2 | [el1[9.1][6.1,7.1]{}] | 1
3 | [el2[10.1][6.1,8.1]{}] | 1
3 | [el3[11.1][7.1,8.1]{},el1[9.1][6.1,7.1]{}] | 1
3 | [el2[10.1][6.1,8.1]{}] | 1
3 | [el3[11.1][7.1,8.1]{}] | 2
(4 rows)

Expand Down Expand Up @@ -659,42 +659,42 @@ MATCH (A)<-[r:el2 ONLY *1..3]-(B) RETURN A.id, r, B.id;
MATCH (A)-[r:el1 *1..3]-(B) RETURN A.id, r, B.id;
id | r | id
----+-----------------------------------------------------------------+----
2 | [el1[9.1][6.1,7.1]{}] | 1
2 | [el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{}] | 1
1 | [el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{}] | 1
3 | [el2[10.1][6.1,8.1]{}] | 1
3 | [el3[11.1][7.1,8.1]{},el1[9.1][6.1,7.1]{}] | 1
1 | [el2[10.1][6.1,8.1]{},el3[11.1][7.1,8.1]{},el1[9.1][6.1,7.1]{}] | 1
1 | [el1[9.1][6.1,7.1]{}] | 2
1 | [el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{}] | 1
1 | [el2[10.1][6.1,8.1]{},el3[11.1][7.1,8.1]{}] | 2
2 | [el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{},el1[9.1][6.1,7.1]{}] | 2
2 | [el1[9.1][6.1,7.1]{},el2[10.1][6.1,8.1]{},el3[11.1][7.1,8.1]{}] | 2
3 | [el2[10.1][6.1,8.1]{},el1[9.1][6.1,7.1]{}] | 2
3 | [el3[11.1][7.1,8.1]{}] | 2
1 | [el1[9.1][6.1,7.1]{}] | 2
1 | [el2[10.1][6.1,8.1]{}] | 3
1 | [el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{}] | 3
3 | [el2[10.1][6.1,8.1]{},el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{}] | 3
2 | [el3[11.1][7.1,8.1]{}] | 3
2 | [el1[9.1][6.1,7.1]{}] | 1
2 | [el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{}] | 1
2 | [el1[9.1][6.1,7.1]{},el2[10.1][6.1,8.1]{},el3[11.1][7.1,8.1]{}] | 2
2 | [el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{},el1[9.1][6.1,7.1]{}] | 2
2 | [el1[9.1][6.1,7.1]{},el2[10.1][6.1,8.1]{}] | 3
2 | [el3[11.1][7.1,8.1]{}] | 3
3 | [el3[11.1][7.1,8.1]{},el1[9.1][6.1,7.1]{}] | 1
3 | [el2[10.1][6.1,8.1]{}] | 1
3 | [el3[11.1][7.1,8.1]{}] | 2
3 | [el2[10.1][6.1,8.1]{},el1[9.1][6.1,7.1]{}] | 2
3 | [el3[11.1][7.1,8.1]{},el1[9.1][6.1,7.1]{},el2[10.1][6.1,8.1]{}] | 3
3 | [el2[10.1][6.1,8.1]{},el1[9.1][6.1,7.1]{},el3[11.1][7.1,8.1]{}] | 3
(18 rows)

MATCH (A)-[r:el2 *1..3]-(B) RETURN A.id, r, B.id;
id | r | id
----+---------------------------------------------+----
2 | [el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{}] | 1
3 | [el2[10.1][6.1,8.1]{}] | 1
1 | [el2[10.1][6.1,8.1]{},el3[11.1][7.1,8.1]{}] | 2
3 | [el3[11.1][7.1,8.1]{}] | 2
1 | [el2[10.1][6.1,8.1]{}] | 3
2 | [el3[11.1][7.1,8.1]{},el2[10.1][6.1,8.1]{}] | 1
2 | [el3[11.1][7.1,8.1]{}] | 3
3 | [el2[10.1][6.1,8.1]{}] | 1
3 | [el3[11.1][7.1,8.1]{}] | 2
(6 rows)

MATCH (A)-[r:el3 *1..3]-(B) RETURN A.id, r, B.id;
id | r | id
----+------------------------+----
3 | [el3[11.1][7.1,8.1]{}] | 2
2 | [el3[11.1][7.1,8.1]{}] | 3
3 | [el3[11.1][7.1,8.1]{}] | 2
(2 rows)

MATCH (A)-[r:el1 ONLY *1..3]-(B) RETURN A.id, r, B.id;
Expand Down Expand Up @@ -2003,9 +2003,9 @@ RETURN DISTINCT c.lang AS c;
"agens-graph-jdbc"
"agens-graph-docs"
"agens-graph-odbc"
"en"
"c"
"java"
"en"
(7 rows)

MATCH (a)
Expand Down

0 comments on commit c05c235

Please sign in to comment.