Skip to content

Commit

Permalink
feat(pg): PG10 identity columns and preliminary PG11 support (#294)
Browse files Browse the repository at this point in the history
Fixes #244 

Uses a template string to support multiple PostgreSQL versions.
  • Loading branch information
mattbretl authored and benjie committed Sep 8, 2018
1 parent 9b651f1 commit 1e040ba
Show file tree
Hide file tree
Showing 11 changed files with 789 additions and 19 deletions.
4 changes: 3 additions & 1 deletion packages/graphile-build-pg/src/plugins/PgColumnsPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export default (function PgColumnsPlugin(builder) {
isPgBaseInput ? "base" : isPgPatch ? "update" : "create"
)
)
.filter(attr => attr.identity !== "a")
.reduce((memo, attr) => {
const fieldName = inflection.column(attr);
if (memo[fieldName]) {
Expand All @@ -248,7 +249,8 @@ export default (function PgColumnsPlugin(builder) {
isPgBaseInput ||
isPgPatch ||
(!attr.isNotNull && !attr.type.domainIsNotNull) ||
attr.hasDefault,
attr.hasDefault ||
attr.identity === "d",
pgGetGqlInputTypeByTypeIdAndModifier(
attr.typeId,
attr.typeModifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export interface PgAttribute {
typeModifier: number;
isNotNull: boolean;
hasDefault: boolean;
identity: "" | "a" | "d";
class: PgClass;
type: PgType;
namespace: PgNamespace;
Expand Down
12 changes: 10 additions & 2 deletions packages/graphile-build-pg/src/plugins/PgIntrospectionPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import chalk from "chalk";
import throttle from "lodash/throttle";
import flatMap from "lodash/flatMap";
import { quacksLikePgPool } from "../withPgClient";
import { makeIntrospectionQuery } from "./introspectionQuery";

import { version } from "../../package.json";

const debug = debugFactory("graphile-build-pg");
const INTROSPECTION_PATH = `${__dirname}/../../res/introspection-query.sql`;
const WATCH_FIXTURES_PATH = `${__dirname}/../../res/watch-fixtures.sql`;

// Ref: https://github.com/graphile/postgraphile/tree/master/src/postgres/introspection/object
Expand Down Expand Up @@ -102,6 +102,7 @@ export type PgAttribute = {
typeModifier: number,
isNotNull: boolean,
hasDefault: boolean,
identity: "" | "a" | "d",
class: PgClass,
type: PgType,
namespace: PgNamespace,
Expand Down Expand Up @@ -175,7 +176,14 @@ export default (async function PgIntrospectionPlugin(
const introspectionResultsByKind = cloneResults(
await persistentMemoizeWithKey(cacheKey, () =>
withPgClient(pgConfig, async pgClient => {
const introspectionQuery = await readFile(INTROSPECTION_PATH, "utf8");
const versionResult = await pgClient.query(
"show server_version_num;"
);
const serverVersionNum = parseInt(
versionResult.rows[0].server_version_num,
10
);
const introspectionQuery = makeIntrospectionQuery(serverVersionNum);
const { rows } = await pgClient.query(introspectionQuery, [
schemas,
pgIncludeExtensionResources,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// @flow
function makeIntrospectionQuery(serverVersionNum: number): string {
return `\
-- @see https://www.postgresql.org/docs/9.5/static/catalogs.html
-- @see https://github.com/graphile/postgraphile/blob/master/resources/introspection-query.sql
--
-- ## Parameters
-- - `$1`: An array of strings that represent the namespaces we are introspecting.
-- - `$2`: set true to include functions/tables/etc that come from extensions
-- - \`$1\`: An array of strings that represent the namespaces we are introspecting.
-- - \`$2\`: set true to include functions/tables/etc that come from extensions
with
recursive accessible_roles(_oid) as (
select oid _oid, pg_roles.*
Expand Down Expand Up @@ -66,18 +69,21 @@ with
-- TODO: Variadic arguments.
pro.provariadic = 0 and
-- Filter our aggregate functions and window functions.
pro.proisagg = false and
pro.proiswindow = false and
${
serverVersionNum >= 110000
? "pro.prokind = 'f'"
: "pro.proisagg = false and pro.proiswindow = false"
} and
-- We want to make sure the argument mode for all of our arguments is
-- `IN` which means `proargmodes` will be null.
-- \`IN\` which means \`proargmodes\` will be null.
pro.proargmodes is null and
-- Do not select procedures that create range types. These are utility
-- functions that really don’t need to be exposed in an API.
pro.proname not in (select typ.typname from pg_catalog.pg_type as typ where typ.typtype = 'r' and typ.typnamespace = pro.pronamespace) and
-- Do not expose trigger functions (type trigger has oid 2279)
pro.prorettype <> 2279 and
-- We don't want functions that will clash with GraphQL (treat them as private)
pro.proname not like E'\\_\\_%' and
pro.proname not like E'\\\\_\\\\_%' and
-- We also don’t want procedures that have been defined in our namespace
-- twice. This leads to duplicate fields in the API which throws an
-- error. In the future we may support this case. For now though, it is
Expand Down Expand Up @@ -106,14 +112,14 @@ with
nsp.nspname as "namespaceName",
rel.reltype as "typeId",
-- Here we determine whether or not we can use this class in a
-- `SELECT`’s `FROM` clause. In order to determine this we look at them
-- `relkind` column, if it is `i` (index) or `c` (composite), we cannot
-- \`SELECT\`’s \`FROM\` clause. In order to determine this we look at them
-- \`relkind\` column, if it is \`i\` (index) or \`c\` (composite), we cannot
-- select this class. Otherwise we can.
rel.relkind not in ('i', 'c') as "isSelectable",
-- Here we are determining whether we can insert/update/delete a class.
-- This is helpful as it lets us detect non-updatable views and then
-- exclude them from being inserted/updated/deleted into. For more info
-- on how `pg_catalog.pg_relation_is_updatable` works:
-- on how \`pg_catalog.pg_relation_is_updatable\` works:
--
-- - https://www.postgresql.org/message-id/CAEZATCV2_qN9P3zbvADwME_TkYf2gR_X2cLQR4R+pqkwxGxqJg@mail.gmail.com
-- - https://github.com/postgres/postgres/blob/2410a2543e77983dab1f63f48b2adcd23dba994e/src/backend/utils/adt/misc.c#L684
Expand All @@ -132,7 +138,7 @@ with
where
rel.relpersistence in ('p') and
-- We don't want classes that will clash with GraphQL (treat them as private)
rel.relname not like E'\\_\\_%' and
rel.relname not like E'\\\\_\\\\_%' and
rel.relkind in ('r', 'v', 'm', 'c', 'f') and
($2 is true or not exists(
select 1
Expand All @@ -157,6 +163,7 @@ with
nullif(att.atttypmod, -1) as "typeModifier",
att.attnotnull as "isNotNull",
att.atthasdef as "hasDefault",
${serverVersionNum >= 100000 ? "att.attidentity" : "''"} as "identity",
exists(select 1 from accessible_roles where has_column_privilege(accessible_roles.oid, att.attrelid, att.attname, 'SELECT')) as "aclSelectable",
exists(select 1 from accessible_roles where has_column_privilege(accessible_roles.oid, att.attrelid, att.attname, 'INSERT')) as "aclInsertable",
exists(select 1 from accessible_roles where has_column_privilege(accessible_roles.oid, att.attrelid, att.attname, 'UPDATE')) as "aclUpdatable"
Expand All @@ -167,14 +174,14 @@ with
att.attrelid in (select "id" from class) and
att.attnum > 0 and
-- We don't want attributes that will clash with GraphQL (treat them as private)
att.attname not like E'\\_\\_%' and
att.attname not like E'\\\\_\\\\_%' and
not att.attisdropped
order by
att.attrelid, att.attnum
),
-- @see https://www.postgresql.org/docs/9.5/static/catalog-pg-type.html
type as (
-- Use another `WITH` statement here, because our `WHERE` clause will need
-- Use another \`WITH\` statement here, because our \`WHERE\` clause will need
-- to use it.
with type_all as (
select
Expand Down Expand Up @@ -311,3 +318,9 @@ select row_to_json(x) as object from procedure as x
union all
select row_to_json(x) as object from extension as x
;
`;
}

module.exports = {
makeIntrospectionQuery,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
mutation {
a: createAlwaysAsIdentity(input:{
alwaysAsIdentity: {
t: "test"
}
}) {
alwaysAsIdentity {
id
t
}
}
b: createByDefaultAsIdentity(input:{
byDefaultAsIdentity: {
t: "test"
}
}) {
byDefaultAsIdentity {
id
t
}
}
c: createByDefaultAsIdentity(input:{
byDefaultAsIdentity: {
id: 100
t: "test"
}
}) {
byDefaultAsIdentity {
id
t
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,31 @@ Object {
}
`;
exports[`pg10.identity.graphql 1`] = `
Object {
"data": Object {
"a": Object {
"alwaysAsIdentity": Object {
"id": 1,
"t": "test",
},
},
"b": Object {
"byDefaultAsIdentity": Object {
"id": 1,
"t": "test",
},
},
"c": Object {
"byDefaultAsIdentity": Object {
"id": 100,
"t": "test",
},
},
},
}
`;
exports[`procedure-mutation.graphql 1`] = `
Object {
"data": Object {
Expand Down
23 changes: 20 additions & 3 deletions packages/postgraphile-core/__tests__/integration/mutations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ function readFile(filename, encoding) {
});
}

async function getServerVersionNum(pgClient) {
const versionResult = await pgClient.query("show server_version_num;");
return parseInt(versionResult.rows[0].server_version_num, 10);
}

// This test suite can be flaky. Increase it’s timeout.
jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 20;

Expand All @@ -31,10 +36,13 @@ beforeAll(() => {
const gqlSchemaPromise = withPgClient(async pgClient => {
// A selection of omit/rename comments on the d schema
await pgClient.query(await dSchemaComments());

const [gqlSchema, dSchema] = await Promise.all([
const serverVersionNum = await getServerVersionNum(pgClient);
const [gqlSchema, dSchema, pg10Schema] = await Promise.all([
createPostGraphileSchema(pgClient, ["a", "b", "c"]),
createPostGraphileSchema(pgClient, ["d"]),
serverVersionNum >= 100000
? createPostGraphileSchema(pgClient, ["pg10"])
: null,
]);
// Now for RBAC-enabled tests
await pgClient.query("set role postgraphile_test_authenticator");
Expand All @@ -44,6 +52,7 @@ beforeAll(() => {
return {
gqlSchema,
dSchema,
pg10Schema,
rbacSchema,
};
});
Expand All @@ -56,7 +65,7 @@ beforeAll(() => {
mutationResults = mutationFileNames.map(async fileName => {
// Wait for the schema to resolve. We need the schema to be introspected
// before we can do anything else!
let { gqlSchema, dSchema, rbacSchema } = await gqlSchemaPromise;
let { gqlSchema, dSchema, pg10Schema, rbacSchema } = await gqlSchemaPromise;
// Get a new Postgres client and run the mutation.
return await withPgClient(async pgClient => {
// Read the mutation from the file system.
Expand All @@ -71,6 +80,14 @@ beforeAll(() => {
let schemaToUse;
if (fileName.startsWith("d.")) {
schemaToUse = dSchema;
} else if (fileName.startsWith("pg10.")) {
const serverVersionNum = await getServerVersionNum(pgClient);
if (serverVersionNum < 100000) {
// eslint-disable-next-line
console.log("Skipping test as PG version is less than 10");
return;
}
schemaToUse = pg10Schema;
} else if (fileName.startsWith("rbac.")) {
await pgClient.query(
"select set_config('role', 'postgraphile_test_visitor', true), set_config('jwt.claims.user_id', '3', true)"
Expand Down
Loading

0 comments on commit 1e040ba

Please sign in to comment.