Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

Improve order by performance #247

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions src/translate.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ export const translateQuery = ({
temporalArgs
);
const outerSkipLimit = getOuterSkipLimit(first, offset);
const orderByValue = computeOrderBy(resolveInfo, selections);
const orderByValue = computeOrderBy(resolveInfo, schemaType);

if (queryTypeCypherDirective) {
return customQuery({
Expand Down Expand Up @@ -706,14 +706,15 @@ const customQuery = ({
});
const isScalarType = isGraphqlScalarType(schemaType);
const temporalType = isTemporalType(schemaType.name);
const { cypherPart: orderByClause } = orderByValue;
const query = `WITH apoc.cypher.runFirstColumn("${
cypherQueryArg.value.value
}", ${argString ||
'null'}, True) AS x UNWIND x AS ${safeVariableName} RETURN ${safeVariableName} ${
// Don't add subQuery for scalar type payloads
// FIXME: fix subselection translation for temporal type payload
!temporalType && !isScalarType
? `{${subQuery}} AS ${safeVariableName}${orderByValue}`
? `{${subQuery}} AS ${safeVariableName}${orderByClause}`
: ''
}${outerSkipLimit}`;
return [query, params];
Expand Down Expand Up @@ -795,11 +796,15 @@ const nodeQuery = ({

const predicate = predicateClauses ? `WHERE ${predicateClauses} ` : '';

let query =
`MATCH (${safeVariableName}:${safeLabelName}${
argString ? ` ${argString}` : ''
}) ${predicate}` +
`RETURN ${safeVariableName} {${subQuery}} AS ${safeVariableName}${orderByValue}${outerSkipLimit}`;
const { optimization, cypherPart: orderByClause } = orderByValue;

let query = `MATCH (${safeVariableName}:${safeLabelName}${
argString ? ` ${argString}` : ''
}) ${predicate}${
optimization.earlyOrderBy ? `WITH ${safeVariableName}${orderByClause}` : ''
}RETURN ${safeVariableName} {${subQuery}} AS ${safeVariableName}${
optimization.earlyOrderBy ? '' : orderByClause
}${outerSkipLimit}`;

return [query, params];
};
Expand All @@ -817,7 +822,7 @@ export const translateMutation = ({
otherParams
}) => {
const outerSkipLimit = getOuterSkipLimit(first, offset);
const orderByValue = computeOrderBy(resolveInfo, selections);
const orderByValue = computeOrderBy(resolveInfo, schemaType);
const mutationTypeCypherDirective = getMutationCypherDirective(resolveInfo);
const params = initializeMutationParams({
resolveInfo,
Expand Down Expand Up @@ -918,13 +923,14 @@ const customMutation = ({
if (cypherParams) {
params['cypherParams'] = cypherParams;
}
const { cypherPart: orderByClause } = orderByValue;
const query = `CALL apoc.cypher.doIt("${
cypherQueryArg.value.value
}", ${argString}) YIELD value
WITH apoc.map.values(value, [keys(value)[0]])[0] AS ${safeVariableName}
RETURN ${safeVariableName} ${
!temporalType && !isScalarType
? `{${subQuery}} AS ${safeVariableName}${orderByValue}${outerSkipLimit}`
? `{${subQuery}} AS ${safeVariableName}${orderByClause}${outerSkipLimit}`
: ''
}`;
return [query, params];
Expand Down
34 changes: 26 additions & 8 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,31 +375,49 @@ export function computeSkipLimit(selection, variableValues) {
return `[${offset}..${parseInt(offset) + parseInt(first)}]`;
}

function orderByStatement(resolveInfo, orderByVar) {
function splitOrderByArg(orderByVar) {
const splitIndex = orderByVar.lastIndexOf('_');
const order = orderByVar.substring(splitIndex + 1);
const orderBy = orderByVar.substring(0, splitIndex);
return { orderBy, order };
}

function orderByStatement(resolveInfo, { orderBy, order }) {
const { variableName } = typeIdentifiers(resolveInfo.returnType);
return ` ${variableName}.${orderBy} ${order === 'asc' ? 'ASC' : 'DESC'} `;
}

export const computeOrderBy = (resolveInfo, selection) => {
export const computeOrderBy = (resolveInfo, schemaType) => {
let selection = resolveInfo.operation.selectionSet.selections[0];
const orderByArgs = argumentValue(
resolveInfo.operation.selectionSet.selections[0],
selection,
'orderBy',
resolveInfo.variableValues
);

if (orderByArgs == undefined) {
return '';
return { cypherPart: '', optimization: { earlyOrderBy: false } };
}

const orderByArray = Array.isArray(orderByArgs) ? orderByArgs : [orderByArgs];
const orderByStatments = orderByArray.map(orderByVar =>
orderByStatement(resolveInfo, orderByVar)
);

return ' ORDER BY' + orderByStatments.join(',');
let optimization = { earlyOrderBy: true };
let orderByStatements = [];

const orderByStatments = orderByArray.map(orderByVar => {
const { orderBy, order } = splitOrderByArg(orderByVar);
const hasNoCypherDirective = _.isEmpty(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be explicit - hasNoCypherDirective here applies to the orderBy field. We should still be able to use the optimized ordering if a @cypher field is requested in the selection set but not in the orderBy arg

cypherDirective(schemaType, orderBy)
);
optimization.earlyOrderBy =
optimization.earlyOrderBy && hasNoCypherDirective;
orderByStatements.push(orderByStatement(resolveInfo, { orderBy, order }));
});

return {
cypherPart: ` ORDER BY${orderByStatements.join(',')}`,
optimization
};
};

export const possiblySetFirstId = ({ args, statements, params }) => {
Expand Down
41 changes: 36 additions & 5 deletions test/cypherTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,7 @@ test('orderBy test - descending, top level - augmented schema', t => {
}
}
`,
expectedCypherQuery = `MATCH (\`movie\`:\`Movie\` {year:$year}) RETURN \`movie\` { .title ,actors: [(\`movie\`)<-[:\`ACTED_IN\`]-(\`movie_actors\`:\`Actor\`) | movie_actors { .name }][..3] } AS \`movie\` ORDER BY movie.title DESC LIMIT $first`;
expectedCypherQuery = `MATCH (\`movie\`:\`Movie\` {year:$year}) WITH \`movie\` ORDER BY movie.title DESC RETURN \`movie\` { .title ,actors: [(\`movie\`)<-[:\`ACTED_IN\`]-(\`movie_actors\`:\`Actor\`) | movie_actors { .name }][..3] } AS \`movie\` LIMIT $first`;

t.plan(1);

Expand Down Expand Up @@ -3860,7 +3860,38 @@ test('Deeply nested orderBy', t => {
}
}`,
expectedCypherQuery =
"MATCH (`movie`:`Movie`) RETURN `movie` { .title ,actors: apoc.coll.sortMulti([(`movie`)<-[:`ACTED_IN`]-(`movie_actors`:`Actor`) | movie_actors { .name ,movies: apoc.coll.sortMulti([(`movie_actors`)-[:`ACTED_IN`]->(`movie_actors_movies`:`Movie`) | movie_actors_movies { .title }], ['^title','title']) }], ['name']) } AS `movie` ORDER BY movie.title DESC ";
"MATCH (`movie`:`Movie`) WITH `movie` ORDER BY movie.title DESC RETURN `movie` { .title ,actors: apoc.coll.sortMulti([(`movie`)<-[:`ACTED_IN`]-(`movie_actors`:`Actor`) | movie_actors { .name ,movies: apoc.coll.sortMulti([(`movie_actors`)-[:`ACTED_IN`]->(`movie_actors_movies`:`Movie`) | movie_actors_movies { .title }], ['^title','title']) }], ['name']) } AS `movie`";

t.plan(1);
return Promise.all([
augmentedSchemaCypherTestRunner(t, graphQLQuery, {}, expectedCypherQuery)
]);
});

test('Optimize performance - not requesting attributes with @cypher directive - early ORDER BY', async t => {
const graphQLQuery = `{
User(orderBy: name_desc) {
_id
name
}
}`,
expectedCypherQuery = `MATCH (\`user\`:\`User\`) WITH \`user\` ORDER BY user.name DESC RETURN \`user\` {_id: ID(\`user\`), .name } AS \`user\``;

t.plan(1);
return Promise.all([
augmentedSchemaCypherTestRunner(t, graphQLQuery, {}, expectedCypherQuery)
]);
});

test('Optimize performance - attributes with @cypher directive requested - no optimization', async t => {
const graphQLQuery = `{
User(orderBy: currentUserId_desc) {
_id
name
currentUserId
}
}`,
expectedCypherQuery = `MATCH (\`user\`:\`User\`) RETURN \`user\` {_id: ID(\`user\`), .name ,currentUserId: apoc.cypher.runFirstColumn("RETURN $cypherParams.currentUserId AS cypherParamsUserId", {this: user, cypherParams: $cypherParams, strArg: "Neo4j"}, false)} AS \`user\` ORDER BY user.currentUserId DESC `;

t.plan(1);
return Promise.all([
Expand All @@ -3876,7 +3907,7 @@ test('Query using enum orderBy', t => {
}
}`,
expectedCypherQuery =
'MATCH (`book`:`Book`) RETURN `book` {_id: ID(`book`), .genre } AS `book` ORDER BY book.genre ASC ';
'MATCH (`book`:`Book`) WITH `book` ORDER BY book.genre ASC RETURN `book` {_id: ID(`book`), .genre } AS `book`';

t.plan(1);
return Promise.all([
Expand All @@ -3895,7 +3926,7 @@ test('Query using temporal orderBy', t => {
}
}`,
expectedCypherQuery =
'MATCH (`temporalNode`:`TemporalNode`) RETURN `temporalNode` {datetime: { formatted: toString(`temporalNode`.datetime) }} AS `temporalNode` ORDER BY temporalNode.datetime DESC , temporalNode.datetime ASC ';
'MATCH (`temporalNode`:`TemporalNode`) WITH `temporalNode` ORDER BY temporalNode.datetime DESC , temporalNode.datetime ASC RETURN `temporalNode` {datetime: { formatted: toString(`temporalNode`.datetime) }} AS `temporalNode`';

t.plan(1);
return Promise.all([
Expand Down Expand Up @@ -3952,7 +3983,7 @@ test('Deeply nested query using temporal orderBy', t => {
}
}`,
expectedCypherQuery =
"MATCH (`temporalNode`:`TemporalNode`) RETURN `temporalNode` {_id: ID(`temporalNode`),datetime: { year: `temporalNode`.datetime.year , month: `temporalNode`.datetime.month , day: `temporalNode`.datetime.day , hour: `temporalNode`.datetime.hour , minute: `temporalNode`.datetime.minute , second: `temporalNode`.datetime.second , millisecond: `temporalNode`.datetime.millisecond , microsecond: `temporalNode`.datetime.microsecond , nanosecond: `temporalNode`.datetime.nanosecond , timezone: `temporalNode`.datetime.timezone , formatted: toString(`temporalNode`.datetime) },temporalNodes: [sortedElement IN apoc.coll.sortMulti([(`temporalNode`)-[:`TEMPORAL`]->(`temporalNode_temporalNodes`:`TemporalNode`) | temporalNode_temporalNodes {_id: ID(`temporalNode_temporalNodes`),datetime: `temporalNode_temporalNodes`.datetime,time: `temporalNode_temporalNodes`.time,temporalNodes: [sortedElement IN apoc.coll.sortMulti([(`temporalNode_temporalNodes`)-[:`TEMPORAL`]->(`temporalNode_temporalNodes_temporalNodes`:`TemporalNode`) | temporalNode_temporalNodes_temporalNodes {_id: ID(`temporalNode_temporalNodes_temporalNodes`),datetime: `temporalNode_temporalNodes_temporalNodes`.datetime,time: `temporalNode_temporalNodes_temporalNodes`.time}], ['datetime','time']) | sortedElement { .*, datetime: {year: sortedElement.datetime.year,formatted: toString(sortedElement.datetime)},time: {hour: sortedElement.time.hour}}][1..3] }], ['^datetime']) | sortedElement { .*, datetime: {year: sortedElement.datetime.year,month: sortedElement.datetime.month,day: sortedElement.datetime.day,hour: sortedElement.datetime.hour,minute: sortedElement.datetime.minute,second: sortedElement.datetime.second,millisecond: sortedElement.datetime.millisecond,microsecond: sortedElement.datetime.microsecond,nanosecond: sortedElement.datetime.nanosecond,timezone: sortedElement.datetime.timezone,formatted: toString(sortedElement.datetime)},time: {hour: sortedElement.time.hour}}] } AS `temporalNode` ORDER BY temporalNode.datetime DESC ";
"MATCH (`temporalNode`:`TemporalNode`) WITH `temporalNode` ORDER BY temporalNode.datetime DESC RETURN `temporalNode` {_id: ID(`temporalNode`),datetime: { year: `temporalNode`.datetime.year , month: `temporalNode`.datetime.month , day: `temporalNode`.datetime.day , hour: `temporalNode`.datetime.hour , minute: `temporalNode`.datetime.minute , second: `temporalNode`.datetime.second , millisecond: `temporalNode`.datetime.millisecond , microsecond: `temporalNode`.datetime.microsecond , nanosecond: `temporalNode`.datetime.nanosecond , timezone: `temporalNode`.datetime.timezone , formatted: toString(`temporalNode`.datetime) },temporalNodes: [sortedElement IN apoc.coll.sortMulti([(`temporalNode`)-[:`TEMPORAL`]->(`temporalNode_temporalNodes`:`TemporalNode`) | temporalNode_temporalNodes {_id: ID(`temporalNode_temporalNodes`),datetime: `temporalNode_temporalNodes`.datetime,time: `temporalNode_temporalNodes`.time,temporalNodes: [sortedElement IN apoc.coll.sortMulti([(`temporalNode_temporalNodes`)-[:`TEMPORAL`]->(`temporalNode_temporalNodes_temporalNodes`:`TemporalNode`) | temporalNode_temporalNodes_temporalNodes {_id: ID(`temporalNode_temporalNodes_temporalNodes`),datetime: `temporalNode_temporalNodes_temporalNodes`.datetime,time: `temporalNode_temporalNodes_temporalNodes`.time}], ['datetime','time']) | sortedElement { .*, datetime: {year: sortedElement.datetime.year,formatted: toString(sortedElement.datetime)},time: {hour: sortedElement.time.hour}}][1..3] }], ['^datetime']) | sortedElement { .*, datetime: {year: sortedElement.datetime.year,month: sortedElement.datetime.month,day: sortedElement.datetime.day,hour: sortedElement.datetime.hour,minute: sortedElement.datetime.minute,second: sortedElement.datetime.second,millisecond: sortedElement.datetime.millisecond,microsecond: sortedElement.datetime.microsecond,nanosecond: sortedElement.datetime.nanosecond,timezone: sortedElement.datetime.timezone,formatted: toString(sortedElement.datetime)},time: {hour: sortedElement.time.hour}}] } AS `temporalNode`";

t.plan(1);
return Promise.all([
Expand Down