Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(totalCount): only query __cursor/rows when needed #40

Merged
merged 13 commits into from
Aug 8, 2017
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
11 changes: 6 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ env:
TEST_DATABASE_URL: postgres://localhost:5432/travis

cache:
yarn: true
directories:
- node_modules
- $NVM_DIR
- packages/graphile-build-pg/node_modules
- packages/graphql-parse-resolve-info/node_modules
- packages/postgraphile-core/node_modules
- packages/graphile-build/node_modules
- $HOME/.yarn

before_install:
- curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 0.28.1
- export PATH="$HOME/.yarn/bin:$PATH"


install:
- yarn
Expand Down
1 change: 1 addition & 0 deletions .yarnrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
workspaces-experimental true
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"babel-plugin-transform-es2015-modules-commonjs": "^6.24.1",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-preset-flow": "^6.23.0",
"flow-bin": "^0.50.0",
"flow-bin": "^0.52.0",
"flow-copy-source": "^1.2.0",
"graphql": "^0.10.5",
"pg": ">=6 < 7",
"sql-formatter": "^1.2.2"
Expand Down
4 changes: 2 additions & 2 deletions packages/graphile-build-pg/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "index.js",
"scripts": {
"test": "jest",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && flow-copy-source src node8plus",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && ../../node_modules/.bin/flow-copy-source src node8plus",
"watch": "mkdir -p node8plus && babel -s true --watch --out-dir node8plus src"
},
"repository": {
Expand Down Expand Up @@ -43,7 +43,7 @@
"lodash": ">=4 <5",
"lru-cache": "4.1.1",
"pg-range-parser": "^1.0.0",
"pg-sql2": "^1.0.0-beta.1",
"pg-sql2": "^1.0.0-beta.3",
"pluralize": "^5.0.0"
},
"peerDependencies": {
Expand Down
4 changes: 4 additions & 0 deletions packages/graphile-build-pg/src/QueryBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ class QueryBuilder {
this.lock("orderBy");
return this.compiledData.orderBy;
}
getSelectFieldsCount() {
this.lockEverything();
return this.compiledData.select.length;
}
buildSelectFields() {
this.lockEverything();
return sql.join(
Expand Down
24 changes: 15 additions & 9 deletions packages/graphile-build-pg/src/plugins/PageInfoStartEndCursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,26 @@ import type { Plugin } from "graphile-build";
export default (function PageInfoStartEndCursor(builder) {
builder.hook(
"GraphQLObjectType:fields",
(fields, { extend, getTypeByName }, { Self }) => {
(fields, { extend, getTypeByName }, { Self, fieldWithHooks }) => {
if (Self.name !== "PageInfo") {
return fields;
}
const Cursor = getTypeByName("Cursor");
return extend(fields, {
startCursor: {
description: "When paginating backwards, the cursor to continue.",
type: Cursor,
},
endCursor: {
description: "When paginating forwards, the cursor to continue.",
type: Cursor,
},
startCursor: fieldWithHooks("startCursor", ({ addDataGenerator }) => {
addDataGenerator(() => ({ usesCursor: [true] }));
return {
description: "When paginating backwards, the cursor to continue.",
type: Cursor,
};
}),
endCursor: fieldWithHooks("endCursor", ({ addDataGenerator }) => {
addDataGenerator(() => ({ usesCursor: [true] }));
return {
description: "When paginating forwards, the cursor to continue.",
type: Cursor,
};
}),
});
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,20 @@ export default (function PgTablesPlugin(builder, { pgInflection: inflection }) {
proc.namespace.name
),
description: `A \`${NodeType.name}\` edge in the connection.`,
fields: () => {
fields: ({ fieldWithHooks }) => {
return {
cursor: {
description: "A cursor for use in pagination.",
type: Cursor,
resolve(data) {
return base64(JSON.stringify(data.__cursor));
},
},
cursor: fieldWithHooks("cursor", ({ addDataGenerator }) => {
addDataGenerator(() => ({
usesCursor: [true],
}));
return {
description: "A cursor for use in pagination.",
type: Cursor,
resolve(data) {
return base64(JSON.stringify(data.__cursor));
},
};
}),
node: {
description: `The \`${NodeType.name}\` at the end of the edge.`,
type: NodeType,
Expand Down
25 changes: 15 additions & 10 deletions packages/graphile-build-pg/src/plugins/PgTablesPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,18 +207,23 @@ export default (function PgTablesPlugin(builder, { pgInflection: inflection }) {
{
description: `A \`${tableTypeName}\` edge in the connection.`,
name: inflection.edge(TableType.name),
fields: ({ recurseDataGeneratorsForField }) => {
fields: ({ fieldWithHooks, recurseDataGeneratorsForField }) => {
recurseDataGeneratorsForField("node");
return {
cursor: {
description: "A cursor for use in pagination.",
type: Cursor,
resolve(data) {
return (
data.__cursor && base64(JSON.stringify(data.__cursor))
);
},
},
cursor: fieldWithHooks("cursor", ({ addDataGenerator }) => {
addDataGenerator(() => ({
usesCursor: [true],
}));
return {
description: "A cursor for use in pagination.",
type: Cursor,
resolve(data) {
return (
data.__cursor && base64(JSON.stringify(data.__cursor))
);
},
};
}),
node: {
description: `The \`${tableTypeName}\` at the end of the edge.`,
type: new GraphQLNonNull(TableType),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ function cursorify(val) {
return val && val.__cursor ? base64(JSON.stringify(val.__cursor)) : val;
}

export default (function(value) {
export default (function addStartEndCursor(value) {
const data = value && value.data && value.data.length ? value.data : null;
const startCursor = cursorify(data && data[0]);
const endCursor = cursorify(data && data[value.data.length - 1]);
Expand Down
4 changes: 2 additions & 2 deletions packages/graphile-build-pg/src/plugins/viaTemporaryTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import sql from "pg-sql2";
import debugFactory from "debug";
import type { Client } from "pg";
import type { SQL, OpaqueSQLQuery } from "pg-sql2";
import type { SQL, SQLQuery } from "pg-sql2";

const debugSql = debugFactory("graphile-build-pg:sql");

Expand Down Expand Up @@ -45,7 +45,7 @@ export default async function viaTemporaryTable(
sqlResultQuery: SQL,
isPgClassLike: boolean = true
) {
async function performQuery(pgClient: Client, sqlQuery: OpaqueSQLQuery) {
async function performQuery(pgClient: Client, sqlQuery: SQLQuery) {
const { text, values } = sql.compile(sqlQuery);
if (debugSql.enabled) debugSql(text);
return pgClient.query(text, values);
Expand Down
73 changes: 46 additions & 27 deletions packages/graphile-build-pg/src/queryFromResolveData.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ export default (
pgCalculateTotalCount,
calculateHasNextPage,
calculateHasPreviousPage,
usesCursor: explicitlyUsesCursor,
} = resolveData;

const usesCursor: boolean =
(explicitlyUsesCursor && explicitlyUsesCursor.length > 0) ||
(calculateHasNextPage && calculateHasNextPage.length > 0) ||
(calculateHasPreviousPage && calculateHasPreviousPage.length > 0) ||
false;
const rawCursorPrefix =
reallyRawCursorPrefix && reallyRawCursorPrefix.filter(_ => _);

Expand Down Expand Up @@ -99,25 +106,27 @@ export default (
options.withCursor
) {
// Sometimes we need a __cursor even if it's not a collection; e.g. to get the edge field on a mutation
queryBuilder.selectCursor(() => {
const orderBy = queryBuilder
.getOrderByExpressionsAndDirections()
.map(([expr]) => expr);
if (orderBy.length > 0) {
return sql.fragment`json_build_array(${sql.join(
[
...getPgCursorPrefix(),
sql.fragment`json_build_array(${sql.join(orderBy, ", ")})`,
],
", "
)})`;
} else {
return sql.fragment`json_build_array(${sql.join(
getPgCursorPrefix(),
", "
)}, (row_number() over (partition by 1)))`;
}
});
if (usesCursor) {
queryBuilder.selectCursor(() => {
const orderBy = queryBuilder
.getOrderByExpressionsAndDirections()
.map(([expr]) => expr);
if (orderBy.length > 0) {
return sql.fragment`json_build_array(${sql.join(
[
...getPgCursorPrefix(),
sql.fragment`json_build_array(${sql.join(orderBy, ", ")})`,
],
", "
)})`;
} else {
return sql.fragment`json_build_array(${sql.join(
getPgCursorPrefix(),
", "
)}, (row_number() over (partition by 1)))`;
}
});
}
}
if (options.withPagination || options.withPaginationAsFields) {
queryBuilder.setCursorComparator((cursorValue, isAfter) => {
Expand Down Expand Up @@ -176,6 +185,7 @@ export default (
});

const query = queryBuilder.build(options);
const haveFields = queryBuilder.getSelectFieldsCount() > 0;
const sqlQueryAlias = sql.identifier(Symbol());
const sqlSummaryAlias = sql.identifier(Symbol());
const canHaveCursorInWhere =
Expand Down Expand Up @@ -212,17 +222,26 @@ export default (
from ${queryBuilder.getTableExpression()} as ${queryBuilder.getTableAlias()}
where ${queryBuilder.buildWhereClause(false, false, options)}
)`;
const sqlWith = sql.fragment`with ${sqlQueryAlias} as (${query}), ${sqlSummaryAlias} as (select json_agg(to_json(${sqlQueryAlias})) as data from ${sqlQueryAlias})`;
const sqlWith = haveFields
? sql.fragment`with ${sqlQueryAlias} as (${query}), ${sqlSummaryAlias} as (select json_agg(to_json(${sqlQueryAlias})) as data from ${sqlQueryAlias})`
: sql.fragment``;
const sqlFrom = sql.fragment``;
const fields = [
[
const fields: Array<[SQL, string]> = [];
if (haveFields) {
fields.push([
sql.fragment`coalesce((select ${sqlSummaryAlias}.data from ${sqlSummaryAlias}), '[]'::json)`,
"data",
],
calculateHasNextPage && [hasNextPage, "hasNextPage"],
calculateHasPreviousPage && [hasPreviousPage, "hasPreviousPage"],
pgCalculateTotalCount && [totalCount, "totalCount"],
].filter(_ => _);
]);
if (calculateHasNextPage) {
fields.push([hasNextPage, "hasNextPage"]);
}
if (calculateHasPreviousPage) {
fields.push([hasPreviousPage, "hasPreviousPage"]);
}
}
if (pgCalculateTotalCount) {
fields.push([totalCount, "totalCount"]);
}
if (options.withPaginationAsFields) {
return sql.fragment`${sqlWith} select ${sql.join(
fields.map(
Expand Down
3 changes: 3 additions & 0 deletions packages/graphile-build/__tests__/fieldData.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ const DummyConnectionPlugin = async builder => {
recurseDataGeneratorsForField,
}) => {
recurseDataGeneratorsForField("node");
addDataGeneratorForField("cursor", () => ({
usesCursor: [true],
}));
addDataGeneratorForField(
"cursor",
(
Expand Down
2 changes: 1 addition & 1 deletion packages/graphile-build/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "index.js",
"scripts": {
"test": "jest",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && flow-copy-source src node8plus",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && ../../node_modules/.bin/flow-copy-source src node8plus",
"watch": "mkdir -p node8plus && babel -s true --watch --out-dir node8plus src"
},
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-parse-resolve-info/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "index.js",
"scripts": {
"test": "jest .",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && flow-copy-source src node8plus",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && ../../node_modules/.bin/flow-copy-source src node8plus",
"watch": "mkdir -p node8plus && babel -s true --watch --out-dir node8plus src"
},
"repository": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
query {
a: allPeople { totalCount }
}
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,16 @@ Object {
}
`;

exports[`connections-totalCount.graphql 1`] = `
Object {
"data": Object {
"a": Object {
"totalCount": 6,
},
},
}
`;

exports[`dynamic-json.graphql 1`] = `
Object {
"data": Object {
Expand Down
2 changes: 1 addition & 1 deletion packages/postgraphile-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "index.js",
"scripts": {
"test": "scripts/test",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && flow-copy-source src node8plus",
"prepublish": "mkdir -p node8plus node7minus && babel -s true --out-dir node8plus src && BABEL_ENV=node7minus babel -s true --out-dir node7minus src && ../../node_modules/.bin/flow-copy-source src node8plus",
"watch": "mkdir -p node8plus && babel -s true --watch --out-dir node8plus src"
},
"repository": {
Expand Down
Loading