Skip to content

Commit

Permalink
feat: validation now works for templated query (pinterest#1119)
Browse files Browse the repository at this point in the history
* feat: validation now works for templated query

* remove isQueryUsingTemplating
  • Loading branch information
czgu authored Jan 6, 2023
1 parent 04f6768 commit 14bd1ab
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 59 deletions.
21 changes: 0 additions & 21 deletions __tests__/lib/templated-query/validation.test.ts

This file was deleted.

13 changes: 10 additions & 3 deletions components/DataDocQueryCell/DataDocQueryCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,11 @@ class DataDocQueryCellComponent extends React.PureComponent<IProps, IState> {
}

@decorate(memoizeOne)
public createGetLintAnnotations(engineId: number) {
return createSQLLinter(engineId);
public createGetLintAnnotations(
engineId: number,
templatedVariables: TDataDocMetaVariables
) {
return createSQLLinter(engineId, templatedVariables);
}

@bind
Expand Down Expand Up @@ -720,7 +723,10 @@ class DataDocQueryCellComponent extends React.PureComponent<IProps, IState> {
onFullScreen={this.props.toggleFullScreen}
getLintErrors={
this.hasQueryValidators
? this.createGetLintAnnotations(this.engineId)
? this.createGetLintAnnotations(
this.engineId,
this.props.templatedVariables
)
: null
}
/>
Expand All @@ -745,6 +751,7 @@ class DataDocQueryCellComponent extends React.PureComponent<IProps, IState> {
templatedVariables={templatedVariables}
engineId={this.engineId}
onRunQueryClick={this.handleRunFromRenderedTemplateModal}
hasValidator={this.hasQueryValidators}
/>
</Modal>
) : null;
Expand Down
32 changes: 21 additions & 11 deletions components/QueryComposer/QueryComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,12 @@ const useRowLimit = (dispatch: Dispatch, environmentId: number) => {
};

const useTemplatedVariables = (dispatch: Dispatch, environmentId: number) => {
const templatedVariables = useSelector((state: IStoreState) => {
const templatedVariablesInState =
state.adhocQuery[environmentId]?.templatedVariables ?? [];

const reduxTemplatedVariables = useSelector(
(state: IStoreState) =>
state.adhocQuery[environmentId]?.templatedVariables
);
const templatedVariables = useMemo(() => {
const templatedVariablesInState = reduxTemplatedVariables ?? [];
if (!Array.isArray(templatedVariablesInState)) {
// This whole block is only here for legacy reason
// In the older version, we are storing it as a dictionary
Expand All @@ -178,7 +180,8 @@ const useTemplatedVariables = (dispatch: Dispatch, environmentId: number) => {
return newConfig;
}
return templatedVariablesInState;
});
}, [reduxTemplatedVariables]);

const setTemplatedVariables = useCallback(
(newVariables: IDataDocMetaVariable[]) =>
dispatch(
Expand Down Expand Up @@ -286,7 +289,10 @@ function useKeyMap(
}, [clickOnRunButton, queryEngines, setEngineId]);
}

function useQueryLint(queryEngine: IQueryEngine) {
function useQueryLint(
queryEngine: IQueryEngine,
templatedVariables: IDataDocMetaVariable[]
) {
const hasQueryValidators = Boolean(queryEngine?.feature_params?.validator);

const getLintAnnotations = useMemo(() => {
Expand All @@ -295,16 +301,16 @@ function useQueryLint(queryEngine: IQueryEngine) {
}

return (query: string, cm: CodeMirror.Editor) =>
createSQLLinter(queryEngine.id)(query, cm);
}, [hasQueryValidators, queryEngine?.id]);
createSQLLinter(queryEngine.id, templatedVariables)(query, cm);
}, [hasQueryValidators, queryEngine?.id, templatedVariables]);

return {
hasQueryValidators,
getLintAnnotations,
};
}

function useTranspileQuery(
query: string,
currentQueryEngine: IQueryEngine,
queryEngines: IQueryEngine[],
setEngineId: (engineId: number) => any,
Expand Down Expand Up @@ -405,14 +411,17 @@ const QueryComposer: React.FC = () => {
}, []);

const { queryEditorRef, handleFormatQuery } = useQueryEditorHelpers();
const { getLintAnnotations } = useQueryLint(engine);
const { getLintAnnotations, hasQueryValidators } = useQueryLint(
engine,
templatedVariables
);
const {
transpilerConfig,
startQueryTranspile,
clearQueryTranspile,
handleTranspileQuery,
transpilerOptions,
} = useTranspileQuery(query, engine, queryEngines, setEngineId, setQuery);
} = useTranspileQuery(engine, queryEngines, setEngineId, setQuery);

const handleCreateDataDoc = useCallback(async () => {
let dataDoc = null;
Expand Down Expand Up @@ -641,6 +650,7 @@ const QueryComposer: React.FC = () => {
setShowRenderedTemplateModal(false);
handleRunQuery();
}}
hasValidator={hasQueryValidators}
/>
</Modal>
);
Expand Down
111 changes: 107 additions & 4 deletions components/TemplateQueryView/TemplatedQueryView.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import React from 'react';
import React, { useMemo } from 'react';

import { TDataDocMetaVariables } from 'const/datadoc';
import { IQueryValidationResult } from 'const/queryExecution';
import { useResource } from 'hooks/useResource';
import { getQueryLinePosition, tokenize } from 'lib/sql-helper/sql-lexer';
import { formatError } from 'lib/utils/error';
import NOOP from 'lib/utils/noop';
import { TemplatedQueryResource } from 'resource/queryExecution';
import { Button } from 'ui/Button/Button';
import { ThemedCodeHighlight } from 'ui/CodeHighlight/ThemedCodeHighlight';
import { ThemedCodeHighlightWithMark } from 'ui/CodeHighlight/ThemedCodeHighlightWithMark';
import { IHighlightRange } from 'ui/CodeHighlight/types';
import { CopyButton } from 'ui/CopyButton/CopyButton';
import { Icon } from 'ui/Icon/Icon';
import { Loading } from 'ui/Loading/Loading';
import { ErrorMessage } from 'ui/Message/ErrorMessage';
import { StyledText } from 'ui/StyledText/StyledText';

import './TemplatedQueryView.scss';

Expand All @@ -17,13 +23,70 @@ export interface ITemplatedQueryViewProps {
templatedVariables: TDataDocMetaVariables;
engineId: number;
onRunQueryClick?: () => void;
hasValidator?: boolean;
}

function useValidateQuery(
renderedQuery: string,
engineId: number,
shouldValidate: boolean
) {
const { data: queryValidationErrors, isLoading } = useResource<
IQueryValidationResult[]
>(
React.useCallback(() => {
if (!shouldValidate) {
// The fake promise is needed because
const fakePromise = Promise.resolve({ data: [] });
(fakePromise as any).cancel = NOOP;
return fakePromise;
}
return TemplatedQueryResource.validateQuery(
renderedQuery,
engineId,
[]
);
}, [shouldValidate, renderedQuery, engineId])
);

const validationErrorHighlights: IHighlightRange[] = useMemo(() => {
if (!queryValidationErrors || queryValidationErrors.length === 0) {
return [];
}
const tokens = tokenize(renderedQuery);
const queryPositions = getQueryLinePosition(renderedQuery);

return queryValidationErrors
.map((error) => {
const token = tokens.find(
(token) =>
token.line === error.line && token.start === error.ch
);
if (token) {
return {
from: queryPositions[token.line] + token.start,
to: queryPositions[token.line] + token.end,
className: 'code-highlight-red',
};
}
return null;
})
.filter((x) => x);
}, [queryValidationErrors, renderedQuery]);

return {
validationErrorHighlights,
queryValidationErrors,
isValidating: isLoading,
};
}

export const TemplatedQueryView: React.FC<ITemplatedQueryViewProps> = ({
query,
templatedVariables,
engineId,
onRunQueryClick,
hasValidator,
}) => {
const {
data: renderedQuery,
Expand All @@ -41,6 +104,13 @@ export const TemplatedQueryView: React.FC<ITemplatedQueryViewProps> = ({
)
);

const { validationErrorHighlights, queryValidationErrors, isValidating } =
useValidateQuery(
renderedQuery,
engineId,
!isLoading && !error && hasValidator
);

let contentDOM: React.ReactNode = null;
if (isLoading) {
contentDOM = <Loading />;
Expand All @@ -52,15 +122,48 @@ export const TemplatedQueryView: React.FC<ITemplatedQueryViewProps> = ({
</ErrorMessage>

<div className="code-wrapper code-error mt16">
<ThemedCodeHighlight value={query} />
<ThemedCodeHighlightWithMark query={query} />
</div>
</div>
);
} else {
const renderQueryValidationErrors = () => {
if (isValidating) {
return (
<div className="flex-row pv8">
<Icon name="Loading" className="mr8" />
<StyledText weight={'bold'}>
Validating query...
</StyledText>
</div>
);
}

if (!queryValidationErrors || queryValidationErrors.length === 0) {
return null;
}

const errorsDOM = queryValidationErrors.map((err, i) => (
<p key={i}>
Line: {err.line} Ch: {err.ch}, Message: {err.message}
</p>
));

return (
<ErrorMessage title={'Query contains validation errors'}>
{errorsDOM}
</ErrorMessage>
);
};

contentDOM = (
<div>
{renderQueryValidationErrors()}
<div className="code-wrapper">
<ThemedCodeHighlight value={renderedQuery} />
<ThemedCodeHighlightWithMark
query={renderedQuery}
highlightRanges={validationErrorHighlights}
/>
</div>
<div className="flex-right mt16">
{onRunQueryClick && (
Expand Down
7 changes: 1 addition & 6 deletions hooks/queryEditor/useLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
ILinterWarning,
TableToken,
} from 'lib/sql-helper/sql-lexer';
import { isQueryUsingTemplating } from 'lib/templated-query/validation';
import { Nullable } from 'lib/typescript';
import React, {
useCallback,
Expand Down Expand Up @@ -81,11 +80,7 @@ function useQueryLintAnnotations(

const getQueryLintAnnotations = useCallback(
async (code: string) => {
if (
!getLintErrors ||
code.length === 0 ||
isQueryUsingTemplating(code)
) {
if (!getLintErrors || code.length === 0) {
return [];
}

Expand Down
12 changes: 10 additions & 2 deletions lib/codemirror/codemirror-lint.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import CodeMirror from 'codemirror';
import { TDataDocMetaVariables } from 'const/datadoc';

import type { ILinterWarning } from 'lib/sql-helper/sql-lexer';
import { TemplatedQueryResource } from 'resource/queryExecution';

export function createSQLLinter(engineId: number) {
export function createSQLLinter(
engineId: number,
templatedVariables: TDataDocMetaVariables
) {
return async (query: string, cm: CodeMirror.Editor) => {
const { data: validationResults } =
await TemplatedQueryResource.validateQuery(query, engineId);
await TemplatedQueryResource.validateQuery(
query,
engineId,
templatedVariables
);

return validationResults.map((validationError) => {
const { line, ch, severity, message } = validationError;
Expand Down
11 changes: 0 additions & 11 deletions lib/templated-query/validation.ts

This file was deleted.

7 changes: 6 additions & 1 deletion resource/queryExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,14 @@ export const TemplatedQueryResource = {
false
),

validateQuery: (query: string, engineId: number) =>
validateQuery: (
query: string,
engineId: number,
templatedVariables: TDataDocMetaVariables
) =>
ds.save<IQueryValidationResult[]>('/query/validate/', {
query,
var_config: templatedVariables,
engine_id: engineId,
}),

Expand Down

0 comments on commit 14bd1ab

Please sign in to comment.