Skip to content

Commit

Permalink
Merge pull request #362 from darrellwarde/revert-union-changes
Browse files Browse the repository at this point in the history 
A new approach for deciding which union members to return!
  • Loading branch information
@darrellwarde
darrellwarde authored Jul 28, 2021
2 parents cc9fe32 + 03247b3 commit 5a81e33
Show file tree
Hide file tree
Showing 9 changed files with 314 additions and 239 deletions.
62 changes: 49 additions & 13 deletions docs/asciidoc/type-definitions/unions-and-interfaces.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ Below you can find some examples of how queries and mutations work with this exa

=== Querying a union

The Neo4j GraphQL Library only returns union members which have inline fragments in your selection set.
Which union members are returned by a Query are dictated by the `where` filter applied.

For example, the following will return users and only their blogs:
For example, the following will return all user content, and you will specifically get the title of each blog.

[source, graphql]
----
Expand All @@ -43,37 +43,73 @@ query GetUsersWithBlogs {
content {
... on Blog {
title
posts {
content
}
}
}
}
}
----

Whilst the query below will return both the blogs and posts of users:
Whilst the query below will only return blogs. We could for instance use a filter to check that the title is not null to essentially return all blogs:

[source, graphql]
----
query GetUsersWithAllContent {
users {
name
content {
content(where: { Blog: { title_NOT: null }}) {
... on Blog {
title
posts {
content
}
}
... on Post {
content
}
}
}
}
----

Conceptually, this maps to the `WHERE` clauses of the subquery unions in Cypher. Going back to the first example with no `where` argument, each subquery has a similar structure:

[source, cypher]
----
CALL {
WITH this
OPTIONAL MATCH (this)-[has_content:HAS_CONTENT]->(blog:Blog)
RETURN { __resolveType: "Blog", title: blog.title }
UNION
WITH this
OPTIONAL MATCH (this)-[has_content:HAS_CONTENT]->(journal:Post)
RETURN { __resolveType: "Post" }
}
----

Now if we were to leave both subqueries and add a `WHERE` clause for blogs, it would look like this:

[source, cypher]
----
CALL {
WITH this
OPTIONAL MATCH (this)-[has_content:HAS_CONTENT]->(blog:Blog)
WHERE blog.title IS NOT NULL
RETURN { __resolveType: "Blog", title: blog.title }
UNION
WITH this
OPTIONAL MATCH (this)-[has_content:HAS_CONTENT]->(journal:Post)
RETURN { __resolveType: "Post" }
}
----

As you can see, the subqueries are now "unbalanced", which could result in massive overfetching of `Post` nodes.

So, when a `where` argument is passed in, we only include union members which are in the `where` object, so it is essentially acting as a logical OR gate, different from the rest of our `where` arguments:

[source, cypher]
----
CALL {
WITH this
OPTIONAL MATCH (this)-[has_content:HAS_CONTENT]->(blog:Blog)
WHERE blog.title IS NOT NULL
RETURN { __resolveType: "Blog", title: blog.title }
}
----

=== Creating a union

The below mutation creates the user and their content:
Expand Down
13 changes: 1 addition & 12 deletions packages/graphql/src/schema/make-augmented-schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1152,20 +1152,9 @@ function makeAugmentedSchema(

const totalCount = isInt(count) ? count.toNumber() : count;

const unionEdges = edges?.filter((edge) => {
if (
Object.keys(edge.node).length === 1 &&
Object.prototype.hasOwnProperty.call(edge.node, "__resolveType")
) {
return false;
}

return true;
});

return {
totalCount,
...createConnectionWithEdgeProperties(unionEdges, args, totalCount),
...createConnectionWithEdgeProperties(edges, args, totalCount),
};
},
},
Expand Down
244 changes: 124 additions & 120 deletions packages/graphql/src/translate/connection/create-connection-and-params.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,143 +89,147 @@ function createConnectionAndParams({
const unionSubqueries: string[] = [];

unionNodes.forEach((n) => {
const relatedNodeVariable = `${nodeVariable}_${n.name}`;
const nodeOutStr = `(${relatedNodeVariable}:${n.name})`;
if (!whereInput || Object.prototype.hasOwnProperty.call(whereInput, n.name)) {
const relatedNodeVariable = `${nodeVariable}_${n.name}`;
const nodeOutStr = `(${relatedNodeVariable}:${n.name})`;

const unionSubquery: string[] = [];
const unionSubqueryElementsToCollect = [...elementsToCollect];
const unionSubquery: string[] = [];
const unionSubqueryElementsToCollect = [...elementsToCollect];

const nestedSubqueries: string[] = [];
const nestedSubqueries: string[] = [];

if (node) {
// Doing this for unions isn't necessary, but this would also work for interfaces if we decided to take that direction
const nodeFieldsByTypeName: FieldsByTypeName = {
[n.name]: {
...node?.fieldsByTypeName[n.name],
...node?.fieldsByTypeName[field.relationship.typeMeta.name],
},
};
if (node) {
const nodeFieldsByTypeName: FieldsByTypeName = {
[n.name]: {
...node?.fieldsByTypeName[n.name],
...node?.fieldsByTypeName[field.relationship.typeMeta.name],
},
};

const nodeProjectionAndParams = createProjectionAndParams({
fieldsByTypeName: nodeFieldsByTypeName,
node: n,
context,
varName: relatedNodeVariable,
literalElements: true,
resolveType: true,
});
const [nodeProjection, nodeProjectionParams] = nodeProjectionAndParams;
unionSubqueryElementsToCollect.push(`node: ${nodeProjection}`);
globalParams = {
...globalParams,
...nodeProjectionParams,
};

if (nodeProjectionAndParams[2]?.connectionFields?.length) {
nodeProjectionAndParams[2].connectionFields.forEach((connectionResolveTree) => {
const connectionField = n.connectionFields.find(
(x) => x.fieldName === connectionResolveTree.name
) as ConnectionField;
const nestedConnection = createConnectionAndParams({
resolveTree: connectionResolveTree,
field: connectionField,
context,
nodeVariable: relatedNodeVariable,
parameterPrefix: `${parameterPrefix ? `${parameterPrefix}.` : `${nodeVariable}_`}${
resolveTree.name
}.edges.node`,
});
nestedSubqueries.push(nestedConnection[0]);

globalParams = {
...globalParams,
...Object.entries(nestedConnection[1]).reduce<Record<string, unknown>>((res, [k, v]) => {
if (k !== `${relatedNodeVariable}_${connectionResolveTree.name}`) {
res[k] = v;
}
return res;
}, {}),
};
const nodeProjectionAndParams = createProjectionAndParams({
fieldsByTypeName: nodeFieldsByTypeName,
node: n,
context,
varName: relatedNodeVariable,
literalElements: true,
resolveType: true,
});
const [nodeProjection, nodeProjectionParams] = nodeProjectionAndParams;
unionSubqueryElementsToCollect.push(`node: ${nodeProjection}`);
globalParams = {
...globalParams,
...nodeProjectionParams,
};

if (nestedConnection[1][`${relatedNodeVariable}_${connectionResolveTree.name}`]) {
if (!nestedConnectionFieldParams) nestedConnectionFieldParams = {};
nestedConnectionFieldParams = {
...nestedConnectionFieldParams,
...{
[connectionResolveTree.name]:
nestedConnection[1][`${relatedNodeVariable}_${connectionResolveTree.name}`],
},
if (nodeProjectionAndParams[2]?.connectionFields?.length) {
nodeProjectionAndParams[2].connectionFields.forEach((connectionResolveTree) => {
const connectionField = n.connectionFields.find(
(x) => x.fieldName === connectionResolveTree.name
) as ConnectionField;
const nestedConnection = createConnectionAndParams({
resolveTree: connectionResolveTree,
field: connectionField,
context,
nodeVariable: relatedNodeVariable,
parameterPrefix: `${parameterPrefix ? `${parameterPrefix}.` : `${nodeVariable}_`}${
resolveTree.name
}.edges.node`,
});
nestedSubqueries.push(nestedConnection[0]);

globalParams = {
...globalParams,
...Object.entries(nestedConnection[1]).reduce<Record<string, unknown>>(
(res, [k, v]) => {
if (k !== `${relatedNodeVariable}_${connectionResolveTree.name}`) {
res[k] = v;
}
return res;
},
{}
),
};
}
});
}
} else {
// This ensures that totalCount calculation is accurate if edges not asked for
unionSubqueryElementsToCollect.push(`node: { __resolveType: "${n.name}" }`);
}

unionSubquery.push(`WITH ${nodeVariable}`);
unionSubquery.push(`OPTIONAL MATCH (${nodeVariable})${inStr}${relTypeStr}${outStr}${nodeOutStr}`);
if (nestedConnection[1][`${relatedNodeVariable}_${connectionResolveTree.name}`]) {
if (!nestedConnectionFieldParams) nestedConnectionFieldParams = {};
nestedConnectionFieldParams = {
...nestedConnectionFieldParams,
...{
[connectionResolveTree.name]:
nestedConnection[1][`${relatedNodeVariable}_${connectionResolveTree.name}`],
},
};
}
});
}
} else {
// This ensures that totalCount calculation is accurate if edges not asked for
unionSubqueryElementsToCollect.push(`node: { __resolveType: "${n.name}" }`);
}

const allowAndParams = createAuthAndParams({
operation: "READ",
entity: n,
context,
allow: {
parentNode: n,
varName: relatedNodeVariable,
},
});
if (allowAndParams[0]) {
globalParams = { ...globalParams, ...allowAndParams[1] };
unionSubquery.push(
`CALL apoc.util.validate(NOT(${allowAndParams[0]}), "${AUTH_FORBIDDEN_ERROR}", [0])`
);
}
unionSubquery.push(`WITH ${nodeVariable}`);
unionSubquery.push(`OPTIONAL MATCH (${nodeVariable})${inStr}${relTypeStr}${outStr}${nodeOutStr}`);

const whereStrs: string[] = [];
const unionWhere = (whereInput || {})[n.name];
if (unionWhere) {
const where = createConnectionWhereAndParams({
whereInput: unionWhere,
node: n,
nodeVariable: relatedNodeVariable,
relationship,
relationshipVariable,
const allowAndParams = createAuthAndParams({
operation: "READ",
entity: n,
context,
parameterPrefix: `${parameterPrefix ? `${parameterPrefix}.` : `${nodeVariable}_`}${
resolveTree.name
}.args.where.${n.name}`,
allow: {
parentNode: n,
varName: relatedNodeVariable,
},
});
const [whereClause] = where;
if (whereClause) {
whereStrs.push(whereClause);
if (allowAndParams[0]) {
globalParams = { ...globalParams, ...allowAndParams[1] };
unionSubquery.push(
`CALL apoc.util.validate(NOT(${allowAndParams[0]}), "${AUTH_FORBIDDEN_ERROR}", [0])`
);
}
}

const whereAuth = createAuthAndParams({
operation: "READ",
entity: n,
context,
where: { varName: relatedNodeVariable, node: n },
});
if (whereAuth[0]) {
whereStrs.push(whereAuth[0]);
globalParams = { ...globalParams, ...whereAuth[1] };
}
const whereStrs: string[] = [];
const unionWhere = (whereInput || {})[n.name];
if (unionWhere) {
const where = createConnectionWhereAndParams({
whereInput: unionWhere,
node: n,
nodeVariable: relatedNodeVariable,
relationship,
relationshipVariable,
context,
parameterPrefix: `${parameterPrefix ? `${parameterPrefix}.` : `${nodeVariable}_`}${
resolveTree.name
}.args.where.${n.name}`,
});
const [whereClause] = where;
if (whereClause) {
whereStrs.push(whereClause);
}
}

if (whereStrs.length) {
unionSubquery.push(`WHERE ${whereStrs.join(" AND ")}`);
}
const whereAuth = createAuthAndParams({
operation: "READ",
entity: n,
context,
where: { varName: relatedNodeVariable, node: n },
});
if (whereAuth[0]) {
whereStrs.push(whereAuth[0]);
globalParams = { ...globalParams, ...whereAuth[1] };
}

if (nestedSubqueries.length) {
unionSubquery.push(nestedSubqueries.join("\n"));
}
if (whereStrs.length) {
unionSubquery.push(`WHERE ${whereStrs.join(" AND ")}`);
}

unionSubquery.push(`WITH { ${unionSubqueryElementsToCollect.join(", ")} } AS edge`);
unionSubquery.push("RETURN edge");
if (nestedSubqueries.length) {
unionSubquery.push(nestedSubqueries.join("\n"));
}

unionSubqueries.push(unionSubquery.join("\n"));
unionSubquery.push(`WITH { ${unionSubqueryElementsToCollect.join(", ")} } AS edge`);
unionSubquery.push("RETURN edge");

unionSubqueries.push(unionSubquery.join("\n"));
}
});

const unionSubqueryCypher = ["CALL {", unionSubqueries.join("\nUNION\n"), "}"];
Expand Down
Loading

0 comments on commit 5a81e33

Please sign in to comment.