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

* Add totalCount query * Declare when cursor is used * Only query cursor when required * Only select fields when necessary * Upgrade flow and pg-sql2 * Less opaqueness * Fix flow issues
graphile · Aug 8, 2017 · f7454f1 · f7454f1
1 parent 1455083
commit f7454f1
Show file tree

Hide file tree

Showing 18 changed files with 154 additions and 96 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -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

diff --git a/.yarnrc b/.yarnrc
@@ -0,0 +1 @@
+workspaces-experimental true
diff --git a/package.json b/package.json
@@ -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"

diff --git a/packages/graphile-build-pg/package.json b/packages/graphile-build-pg/package.json
@@ -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": {
@@ -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": {

diff --git a/packages/graphile-build-pg/src/QueryBuilder.js b/packages/graphile-build-pg/src/QueryBuilder.js
@@ -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(

diff --git a/packages/graphile-build-pg/src/plugins/PageInfoStartEndCursor.js b/packages/graphile-build-pg/src/plugins/PageInfoStartEndCursor.js
@@ -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,
+          };
+        }),
       });
     }
   );

diff --git a/packages/graphile-build-pg/src/plugins/PgScalarFunctionConnectionPlugin.js b/packages/graphile-build-pg/src/plugins/PgScalarFunctionConnectionPlugin.js
@@ -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,

diff --git a/packages/graphile-build-pg/src/plugins/PgTablesPlugin.js b/packages/graphile-build-pg/src/plugins/PgTablesPlugin.js
@@ -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),

diff --git a/packages/graphile-build-pg/src/plugins/addStartEndCursor.js b/packages/graphile-build-pg/src/plugins/addStartEndCursor.js
@@ -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]);

diff --git a/packages/graphile-build-pg/src/plugins/viaTemporaryTable.js b/packages/graphile-build-pg/src/plugins/viaTemporaryTable.js
@@ -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");
 
@@ -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);

diff --git a/packages/graphile-build-pg/src/queryFromResolveData.js b/packages/graphile-build-pg/src/queryFromResolveData.js
@@ -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(_ => _);
 
@@ -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) => {
@@ -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 =
@@ -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(

diff --git a/packages/graphile-build/__tests__/fieldData.test.js b/packages/graphile-build/__tests__/fieldData.test.js
@@ -149,6 +149,9 @@ const DummyConnectionPlugin = async builder => {
                                   recurseDataGeneratorsForField,
                                 }) => {
                                   recurseDataGeneratorsForField("node");
+                                  addDataGeneratorForField("cursor", () => ({
+                                    usesCursor: [true],
+                                  }));
                                   addDataGeneratorForField(
                                     "cursor",
                                     (

diff --git a/packages/graphile-build/package.json b/packages/graphile-build/package.json
@@ -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": {

diff --git a/packages/graphql-parse-resolve-info/package.json b/packages/graphql-parse-resolve-info/package.json
@@ -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": {

diff --git a/packages/postgraphile-core/__tests__/fixtures/queries/connections-totalCount.graphql b/packages/postgraphile-core/__tests__/fixtures/queries/connections-totalCount.graphql
@@ -0,0 +1,3 @@
+query {
+  a: allPeople { totalCount }
+}
diff --git a/packages/postgraphile-core/__tests__/integration/__snapshots__/queries.test.js.snap b/packages/postgraphile-core/__tests__/integration/__snapshots__/queries.test.js.snap
@@ -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 {

diff --git a/packages/postgraphile-core/package.json b/packages/postgraphile-core/package.json
@@ -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": {