fix(nodeId): fix object IDs for arbitrary precision numbers and ints …

…> 9 quadrillion (#493)

packages/graphile-build-pg/src/QueryBuilder.js

@@ -3,7 +3,7 @@ import * as sql from "pg-sql2";
 import type { SQL } from "pg-sql2";
 import isSafeInteger from "lodash/isSafeInteger";
 import chunk from "lodash/chunk";
-import type { PgClass } from "./plugins/PgIntrospectionPlugin";
+import type { PgClass, PgType } from "./plugins/PgIntrospectionPlugin";
 
 // eslint-disable-next-line flowtype/no-weak-types
 type GraphQLContext = any;
@@ -45,6 +45,26 @@ export type QueryBuilderOptions = {
   supportsJSONB?: boolean, // Defaults to true
 };
 
+function escapeLarge(sqlFragment: SQL, type: PgType) {
+  const actualType = type.domainBaseType || type;
+  if (actualType.category === "N") {
+    if (
+      [
+        "21" /* int2 */,
+        "23" /* int4 */,
+        "700" /* float4 */,
+        "701" /* float8 */,
+      ].includes(actualType.id)
+    ) {
+      // No need for special handling
+      return sqlFragment;
+    }
+    // Otherwise force the id to be a string
+    return sql.fragment`((${sqlFragment})::numeric)::text`;
+  }
+  return sqlFragment;
+}
+
 class QueryBuilder {
   parentQueryBuilder: QueryBuilder | void;
   context: GraphQLContext;
@@ -366,9 +386,11 @@ ${sql.join(
     const primaryKeys = primaryKey.keyAttributes;
     this.select(
       sql.fragment`json_build_array(${sql.join(
-        primaryKeys.map(
-          key =>
-            sql.fragment`${this.getTableAlias()}.${sql.identifier(key.name)}`
+        primaryKeys.map(key =>
+          escapeLarge(
+            sql.fragment`${this.getTableAlias()}.${sql.identifier(key.name)}`,
+            key.type
+          )
         ),
         ", "
       )})`,

packages/graphile-build-pg/src/plugins/PgIntrospectionPlugin.d.ts

@@ -79,6 +79,7 @@ export interface PgType {
   classId: string | void;
   class: PgClass | void;
   domainBaseTypeId: string | void;
+  domainBaseType: PgType | void;
   domainTypeModifier: number | void;
   tags: { [tag: string]: true | string | Array<string> };
 }

packages/graphile-build-pg/src/plugins/PgIntrospectionPlugin.js

@@ -101,6 +101,7 @@ export type PgType = {
   classId: ?string,
   class: ?PgClass,
   domainBaseTypeId: ?string,
+  domainBaseType: ?PgType,
   domainTypeModifier: ?number,
   tags: { [string]: string },
 };

packages/graphile-build-pg/src/plugins/PgTablesPlugin.js

@@ -131,12 +131,42 @@ export default (function PgTablesPlugin(
                         "A globally unique identifier. Can be used in various places throughout the system to identify this single value.",
                       type: new GraphQLNonNull(GraphQLID),
                       resolve(data) {
-                        return (
-                          data.__identifiers &&
-                          getNodeIdForTypeAndIdentifiers(
-                            Self,
-                            ...data.__identifiers
-                          )
+                        const identifiers = data.__identifiers;
+                        if (!identifiers) {
+                          return null;
+                        }
+                        /*
+                         * For bigint we want NodeIDs to be the same as int up
+                         * to the limits of int, and only to be strings after
+                         * that point.
+                         */
+                        const finalIdentifiers = identifiers.map(
+                          (identifier, idx) => {
+                            const key = primaryKeys[idx];
+                            const type = key.type.domainBaseType || key.type;
+                            if (type.id === "20" /* bigint */) {
+                              /*
+                               * When migrating from 'int' to 'bigint' we want
+                               * to maintain nodeIDs in the safe range before
+                               * moving to strings for larger numbers. Since we
+                               * can represent ints up to MAX_SAFE_INTEGER
+                               * (2^53 - 1) fine, we're using that as the
+                               * boundary.
+                               */
+                              const int = parseInt(identifier, 10);
+                              if (
+                                int >= -Number.MAX_SAFE_INTEGER &&
+                                int <= Number.MAX_SAFE_INTEGER
+                              ) {
+                                return int;
+                              }
+                            }
+                            return identifier;
+                          }
+                        );
+                        return getNodeIdForTypeAndIdentifiers(
+                          Self,
+                          ...finalIdentifiers
                         );
                       },
                     };

packages/postgraphile-core/__tests__/fixtures/queries/large_bigint.issue491.graphql

@@ -0,0 +1,19 @@
+{
+  allLargeNodeIds {
+    nodes {
+      nodeId
+      id
+      text
+    }
+  }
+  safeInt: largeNodeId(nodeId: "WyJsYXJnZV9ub2RlX2lkcyIsOTAwNzE5OTI1NDc0MDk5MF0=") {
+    nodeId
+    id
+    text
+  }
+  largeInt: largeNodeId(nodeId: "WyJsYXJnZV9ub2RlX2lkcyIsIjIwOTgyODg2NjkyMTg1NzE3NjAiXQ==") {
+    nodeId
+    id
+    text
+  }
+}

packages/postgraphile-core/__tests__/integration/__snapshots__/queries.test.js.snap

@@ -1815,6 +1815,37 @@ Object {
 }
 `;
 
+exports[`large_bigint.issue491.graphql 1`] = `
+Object {
+  "data": Object {
+    "allLargeNodeIds": Object {
+      "nodes": Array [
+        Object {
+          "id": "9007199254740990",
+          "nodeId": "WyJsYXJnZV9ub2RlX2lkcyIsOTAwNzE5OTI1NDc0MDk5MF0=",
+          "text": "Should be fine",
+        },
+        Object {
+          "id": "2098288669218571760",
+          "nodeId": "WyJsYXJnZV9ub2RlX2lkcyIsIjIwOTgyODg2NjkyMTg1NzE3NjAiXQ==",
+          "text": "Graphile Engine issue #491",
+        },
+      ],
+    },
+    "largeInt": Object {
+      "id": "2098288669218571760",
+      "nodeId": "WyJsYXJnZV9ub2RlX2lkcyIsIjIwOTgyODg2NjkyMTg1NzE3NjAiXQ==",
+      "text": "Graphile Engine issue #491",
+    },
+    "safeInt": Object {
+      "id": "9007199254740990",
+      "nodeId": "WyJsYXJnZV9ub2RlX2lkcyIsOTAwNzE5OTI1NDc0MDk5MF0=",
+      "text": "Should be fine",
+    },
+  },
+}
+`;
+
 exports[`longAliases.graphql 1`] = `
 Object {
   "data": Object {

packages/postgraphile-core/__tests__/integration/queries.test.js

@@ -54,6 +54,7 @@ beforeAll(() => {
       simpleCollections,
       orderByNullsLast,
       smartCommentRelations,
+      largeBigint,
     ] = await Promise.all([
       createPostGraphileSchema(pgClient, ["a", "b", "c"], {
         subscriptions: true,
@@ -89,6 +90,7 @@ beforeAll(() => {
         },
       }),
       createPostGraphileSchema(pgClient, ["smart_comment_relations"], {}),
+      createPostGraphileSchema(pgClient, ["large_bigint"], {}),
     ]);
     // Now for RBAC-enabled tests
     await pgClient.query("set role postgraphile_test_authenticator");
@@ -109,6 +111,7 @@ beforeAll(() => {
       orderByNullsLast,
       rbac,
       smartCommentRelations,
+      largeBigint,
     };
   });
 
@@ -164,6 +167,8 @@ beforeAll(() => {
               gqlSchema = gqlSchemas.rbac;
             } else if (fileName.startsWith("smart_comment_relations.")) {
               gqlSchema = gqlSchemas.smartCommentRelations;
+            } else if (fileName.startsWith("large_bigint")) {
+              gqlSchema = gqlSchemas.largeBigint;
             } else {
               gqlSchema = gqlSchemas.normal;
             }

packages/postgraphile-core/__tests__/kitchen-sink-data.sql

@@ -215,3 +215,6 @@ insert into smart_comment_relations.buildings (id, property_id, name, floors, is
   (3, 3, 'Our shed', 1, false),
   (4, 1, 'Home sweet home', 2, true),
   (5, 4, 'The Tower', 200, true);
+
+insert into large_bigint.large_node_id (id, text) values (9007199254740990, 'Should be fine');
+insert into large_bigint.large_node_id (id, text) values (2098288669218571760, 'Graphile Engine issue #491');

packages/postgraphile-core/__tests__/kitchen-sink-schema.sql

@@ -1,5 +1,5 @@
 -- WARNING: this database is shared with graphile-utils, don't run the tests in parallel!
-drop schema if exists a, b, c, d, inheritence, smart_comment_relations, ranges, index_expressions, simple_collections, live_test cascade;
+drop schema if exists a, b, c, d, inheritence, smart_comment_relations, ranges, index_expressions, simple_collections, live_test, large_bigint cascade;
 drop extension if exists tablefunc;
 drop extension if exists intarray;
 drop extension if exists hstore;
@@ -8,6 +8,7 @@ create schema a;
 create schema b;
 create schema c;
 create schema d;
+create schema large_bigint;
 
 alter default privileges revoke execute on functions from public;
 
@@ -1048,3 +1049,8 @@ create table live_test.todos_log_viewed (
   viewed_at timestamp not null default now(),
   foreign key (user_id, todo_id) references live_test.todos_log(user_id, todo_id) on delete cascade
 );
+
+create table large_bigint.large_node_id (
+  id bigint primary key,
+  text text
+);