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

Add support for EXPLAIN options to GraphiQL #1618

Closed
wants to merge 11 commits into from
97 changes: 85 additions & 12 deletions postgraphiql/src/components/PostGraphiQL.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React from 'react';
import GraphiQL from 'graphiql';
import { getOperationAST, parse } from 'graphql';
import {buildClientSchema, getIntrospectionQuery, getOperationAST, GraphQLObjectType, isType, parse} from 'graphql';
Copy link
Member

Choose a reason for hiding this comment

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

I do not understand why prettier is not formatting this file in the way I'd expect. It doesn't seem to be in the prettierignore file, and it doesn't seem to be excluded from the prettier glob

https://github.com/graphile/postgraphile/blob/55bff41460b113481c8161ef8f178f5af0a17df3/package.json#L48

And yet, CI passes. Very strange. I will have to investigate at some point.

import GraphiQLExplorer from 'graphiql-explorer';
import StorageAPI from 'graphiql/dist/utility/StorageAPI';
import './postgraphiql.css';
import { buildClientSchema, getIntrospectionQuery, isType, GraphQLObjectType } from 'graphql';
import { SubscriptionClient } from 'subscriptions-transport-ws';
import { createClient } from 'graphql-ws';
import {SubscriptionClient} from 'subscriptions-transport-ws';
import {createClient} from 'graphql-ws';
import formatSQL from '../formatSQL';

const defaultQuery = `\
Expand Down Expand Up @@ -81,8 +80,24 @@ const STORAGE_KEYS = {
SAVE_HEADERS_TEXT: 'PostGraphiQL:saveHeadersText',
HEADERS_TEXT: 'PostGraphiQL:headersText',
EXPLAIN: 'PostGraphiQL:explain',
EXPLAIN_OPTIONS: 'PostGraphiQL:explainOptions',
};

function explainOptionRequiresAnalyze(option) {
return ['wal', 'timing'].includes(option);
}

function buildExplainOptionsHeader(explainOptions) {
const analyzeEnabled = explainOptions.analyze;
return Object.entries(explainOptions)
.filter(([option]) => !explainOptionRequiresAnalyze(option) || analyzeEnabled)
.map(([option, value]) => `${option}=${value}`).join(',')
}

function writeToClipboard(content) {
return navigator.clipboard.writeText(content);
}

/**
* The standard GraphiQL interface wrapped with some PostGraphile extensions.
* Including a JWT setter and live schema udpate capabilities.
Expand All @@ -101,12 +116,22 @@ class PostGraphiQL extends React.PureComponent {
headersText: this._storage.get(STORAGE_KEYS.HEADERS_TEXT) || '{\n"Authorization": null\n}\n',
explain: this._storage.get(STORAGE_KEYS.EXPLAIN) === 'true',
explainResult: null,
explainOptions: this.parseFromStorage(STORAGE_KEYS.EXPLAIN_OPTIONS) || {
costs: true,
timing: true,
format: 'text'
},
headersTextValid: true,
explorerIsOpen: this._storage.get('explorerIsOpen') === 'false' ? false : true,
explorerIsOpen: this._storage.get('explorerIsOpen') !== 'false',
haveActiveSubscription: false,
socketStatus: POSTGRAPHILE_CONFIG.websockets === 'none' ? null : 'pending',
};

parseFromStorage(key) {
const value = this._storage.get(key);
return value ? JSON.parse(value) : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be wrapped with a try/catch

}

restartRequested = false;
restartSubscriptionsClient = () => {
// implementation will be replaced...
Expand Down Expand Up @@ -375,6 +400,7 @@ class PostGraphiQL extends React.PureComponent {
...(this.state.explain && POSTGRAPHILE_CONFIG.allowExplain
? { 'X-PostGraphile-Explain': 'on' }
: null),
'X-PostGraphile-Explain-Options': buildExplainOptionsHeader(this.state.explainOptions)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should just replace the { 'X-PostGraphile-Explain': 'on' } header with

{ 'X-PostGraphile-Explain': `on; ${buildExplainOptionsHeader(this.state.explainOptions)}` }

?

},
extraHeaders,
),
Expand Down Expand Up @@ -663,6 +689,22 @@ class PostGraphiQL extends React.PureComponent {
);
};

handleToggleExplainOption = (option) => {
this.handleSetExplainOption(option, !this.state.explainOptions[option]);
};

handleSetExplainOption = (option, value) => {
this.setState(
oldState => ({
explainOptions: {
...oldState.explainOptions,
[option]: value
}
}),
() => this._storage.set(STORAGE_KEYS.EXPLAIN_OPTIONS, JSON.stringify(this.state.explainOptions))
);
};

renderSocketStatus() {
const { socketStatus, error } = this.state;
if (socketStatus === null) {
Expand Down Expand Up @@ -815,11 +857,38 @@ class PostGraphiQL extends React.PureComponent {
/>

{POSTGRAPHILE_CONFIG.allowExplain ? (
<GraphiQL.Button
label={this.state.explain ? 'Explain ON' : 'Explain disabled'}
title="View the SQL statements that this query invokes"
onClick={this.handleToggleExplain}
/>
<GraphiQL.Group>
<GraphiQL.Button
label={`Explain ${this.state.explain ? 'ON' : 'OFF'}`}
title="View the SQL statements that this query invokes"
onClick={this.handleToggleExplain}
/>
<GraphiQL.Menu label="Options" title="">
{['analyze', 'verbose', 'costs', 'settings', 'buffers', 'wal', 'timing', 'summary']
.map(option => {
const enabled = !!this.state.explainOptions[option];
const icon = enabled ? '\u2611' : '\u2610';
const requiresAnalyze = explainOptionRequiresAnalyze(option);
const warningIndicator = enabled && requiresAnalyze && !this.state.explainOptions.analyze ? '*' : '';
return <GraphiQL.MenuItem
key={option}
label={`${icon} ${warningIndicator}${option.toUpperCase()}`}
title={requiresAnalyze ? 'Requires ANALYZE' : undefined}
onSelect={() => this.handleToggleExplainOption(option)}
/>;
}
)}
</GraphiQL.Menu>
<GraphiQL.Menu label="Format" title="">
{['text', 'json', 'yaml', 'xml'].map(format =>
<GraphiQL.MenuItem
key={format}
label={`${format.toUpperCase()} ${this.state.explainOptions.format === format ? '\u2713' : ''}`}
onSelect={() => this.handleSetExplainOption('format', format)}
/>
)}
</GraphiQL.Menu>
</GraphiQL.Group>
) : null}
<GraphiQL.Button
label={'Headers ' + (this.state.saveHeadersText ? 'SAVED' : 'unsaved')}
Expand All @@ -838,12 +907,16 @@ class PostGraphiQL extends React.PureComponent {
<a href="https://www.postgresql.org/docs/current/sql-explain.html">
EXPLAIN
</a>{' '}
on executed query:
on executed query:{' '}
<button className="copy-button" onClick={() => writeToClipboard(res.plan)}>copy</button>
</h4>
<pre className="explain-plan">
<code>{res.plan}</code>
</pre>
<h4>Executed SQL query:</h4>
<h4>
Executed SQL query:{' '}
<button className="copy-button" onClick={() => writeToClipboard(res.query)}>copy</button>
</h4>
<pre className="explain-sql">
<code>{formatSQL(res.query)}</code>
</pre>
Expand Down
13 changes: 13 additions & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,18 @@ export interface HttpRequestHandler<
release: () => Promise<void>;
}

export type ExplainOptions = {
analyze?: boolean;
verbose?: boolean;
costs?: boolean;
settings?: boolean;
buffers?: boolean;
wal?: boolean;
timing?: boolean;
summary?: boolean;
format?: 'text' | 'xml' | 'json' | 'yaml';
};

/**
* Options passed to the `withPostGraphileContext` function
*/
Expand All @@ -397,6 +409,7 @@ export interface WithPostGraphileContextOptions {
pgDefaultRole?: string;
pgSettings?: { [key: string]: mixed };
explain?: boolean;
explainOptions: ExplainOptions;
queryDocumentAst?: DocumentNode;
operationName?: string;
pgForceTransaction?: boolean;
Expand Down
37 changes: 36 additions & 1 deletion src/postgraphile/http/createPostGraphileHttpRequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ import {
import { extendedFormatError } from '../extendedFormatError';
import { IncomingMessage, ServerResponse } from 'http';
import { pluginHookFromOptions } from '../pluginHook';
import { HttpRequestHandler, mixed, CreateRequestHandlerOptions } from '../../interfaces';
import {
HttpRequestHandler,
mixed,
CreateRequestHandlerOptions,
ExplainOptions,
} from '../../interfaces';
import setupServerSentEvents from './setupServerSentEvents';
import withPostGraphileContext from '../withPostGraphileContext';
import LRU from '@graphile/lru';
Expand Down Expand Up @@ -110,6 +115,34 @@ const isPostGraphileDevelopmentMode = process.env.POSTGRAPHILE_ENV === 'developm
const debugGraphql = Debugger('postgraphile:graphql');
const debugRequest = Debugger('postgraphile:request');

function parseExplainOptions(req: IncomingMessage): ExplainOptions {
const explainOptions = {};
const header = req.headers['x-postgraphile-explain-options'];
if (!header) {
return explainOptions;
}
const headerValues = typeof header === 'string' ? header.split(',') : header;
for (const headerValue of headerValues) {
const [option, value] = headerValue.split('=').map(s => s.toLowerCase().trim());
switch (option) {
case 'format':
if (['text', 'xml', 'json', 'yaml'].includes(value)) {
explainOptions[option] = value;
} else {
console.warn(`Ignoring invalid value '${value}' for explain option '${option}'.`);
}
break;
default:
if (['true', 'false'].includes(value)) {
explainOptions[option] = value === 'true';
} else {
console.warn(`Ignoring invalid value '${value}' for explain option '${option}'.`);
}
}
}
return explainOptions;
}

/**
* We need to be able to share the withPostGraphileContext logic between HTTP
* and websockets
Expand Down Expand Up @@ -150,6 +183,7 @@ function withPostGraphileContextFromReqResGenerator(
jwtToken,
pgSettings,
explain: allowExplain && req.headers['x-postgraphile-explain'] === 'on',
explainOptions: parseExplainOptions(req),
Copy link
Member

Choose a reason for hiding this comment

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

Let's merge explain/explainOptions together. We don't need to support any legacy here since PostGraphile and PostGraphiQL are deeply integrated and tied to the exact same versions, so unified changes are fine - no need for backwards compatibility.

Suggested change
explain: allowExplain && req.headers['x-postgraphile-explain'] === 'on',
explainOptions: parseExplainOptions(req),
explain: allowExplain && parseExplainOptions(req),

...moreOptions,
},
context => {
Expand Down Expand Up @@ -1175,6 +1209,7 @@ function addCORSHeaders(res: PostGraphileResponse): void {
'Content-Length',
// For our 'Explain' feature
'X-PostGraphile-Explain',
'X-PostGraphile-Explain-Options',
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it, I'm very keen that we don't introduce another header because it means people may have to rewrite their nginx, CORS and similar config.

].join(', '),
);
res.setHeader('Access-Control-Expose-Headers', ['X-GraphQL-Event-Stream'].join(', '));
Expand Down
52 changes: 41 additions & 11 deletions src/postgraphile/withPostGraphileContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ExecutionResult, OperationDefinitionNode, Kind } from 'graphql';
import * as sql from 'pg-sql2';
import { $$pgClient } from '../postgres/inventory/pgClientFromContext';
import { pluginHookFromOptions } from './pluginHook';
import { mixed, WithPostGraphileContextOptions } from '../interfaces';
import { ExplainOptions, mixed, WithPostGraphileContextOptions } from '../interfaces';
import { formatSQLForDebugging } from 'postgraphile-core';

const undefinedIfEmpty = (
Expand Down Expand Up @@ -78,6 +78,7 @@ const withDefaultPostGraphileContext: WithPostGraphileContextFn = async (
pgDefaultRole,
pgSettings,
explain,
explainOptions,
queryDocumentAst,
operationName,
pgForceTransaction,
Expand Down Expand Up @@ -214,7 +215,7 @@ const withDefaultPostGraphileContext: WithPostGraphileContextFn = async (
return withAuthenticatedPgClient(async pgClient => {
let results: Promise<Array<ExplainResult>> | null = null;
if (explain) {
pgClient.startExplain();
pgClient.startExplain(explainOptions);
}
try {
return await callback({
Expand Down Expand Up @@ -454,7 +455,8 @@ type ExplainResult = Omit<RawExplainResult, 'result'> & {
declare module 'pg' {
interface ClientBase {
_explainResults: Array<RawExplainResult> | null;
startExplain: () => void;
_explainOptions: ExplainOptions | null;
startExplain: (options: ExplainOptions) => void;
stopExplain: () => Promise<Array<ExplainResult>>;
}
}
Expand All @@ -471,13 +473,15 @@ export function debugPgClient(pgClient: PoolClient, allowExplain = false): PoolC
// already set, use that.
pgClient[$$pgClientOrigQuery] = pgClient.query;

pgClient.startExplain = () => {
pgClient.startExplain = options => {
pgClient._explainResults = [];
pgClient._explainOptions = options;
};

pgClient.stopExplain = async () => {
const results = pgClient._explainResults;
pgClient._explainResults = null;
pgClient._explainOptions = null;
if (!results) {
return Promise.resolve([]);
}
Expand All @@ -490,7 +494,10 @@ export function debugPgClient(pgClient: PoolClient, allowExplain = false): PoolC
if (!firstKey) {
return null;
}
const plan = result.map((r: any) => r[firstKey]).join('\n');
const plan = result
.map((r: any) => r[firstKey])
.map((r: any) => (typeof r === 'string' ? r : JSON.stringify(r, null, 2)))
.join('\n');
return {
...rest,
plan,
Expand Down Expand Up @@ -532,15 +539,38 @@ export function debugPgClient(pgClient: PoolClient, allowExplain = false): PoolC
const query = a && a.text ? a.text : a;
const values = a && a.text ? a.values : b;
if (query.match(/^\s*(select|insert|update|delete|with)\s/i) && !query.includes(';')) {
// Build the EXPLAIN command
let explainCommand = 'explain';
const explainOptionClauses = Object.entries(pgClient._explainOptions ?? {}).map(
([option, value]) => `${option} ${value}`,
Copy link
Member

Choose a reason for hiding this comment

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

_explainOptions isn't safe to use like this, this opens PostGraphile up to SQL injection attacks.

);
if (explainOptionClauses.length > 0) {
explainCommand = `${explainCommand} (${explainOptionClauses.join(', ')})`;
}
// Explain it
const explain = `explain ${query}`;
const explain = `${explainCommand} ${query}`;

// IMPORTANT: the following 3 queries MUST BE ISSUED SYNCHRONOUSLY to ensure other concurrent queries
// aren't interleaved.

// Create a savepoint before running the EXPLAIN, so we can roll back to it to avoid running mutations
// twice when ANALYZE is enabled.
// This query will fail if there isn't an active transaction. That's fine because that means this is a
// query, so we don't need to roll it back anyway.
pgClient[$$pgClientOrigQuery]
.call(this, 'savepoint postgraphile_explain')
.catch(() => {});
const explainPromise = pgClient[$$pgClientOrigQuery]
.call(this, explain, values)
.then((data: any) => data.rows)
// swallow errors during explain
.catch(() => null);
pgClient[$$pgClientOrigQuery]
.call(this, 'rollback to savepoint postgraphile_explain')
.catch(() => {});
pgClient._explainResults.push({
query,
result: pgClient[$$pgClientOrigQuery]
.call(this, explain, values)
.then((data: any) => data.rows)
// swallow errors during explain
.catch(() => null),
result: explainPromise,
});
}
}
Expand Down