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

Fix(wren-ui): Fix getting incorrect native sql issue #550

Merged
merged 1 commit into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ export interface IbisBaseOptions {
export interface IbisQueryOptions extends IbisBaseOptions {
limit?: number;
}
export interface IbisDryPlanOptions {
dataSource: DataSourceName;
mdl: Manifest;
sql: string;
}

export interface IIbisAdaptor {
query: (
Expand All @@ -102,6 +107,7 @@ export interface IIbisAdaptor {
connectionInfo: WREN_AI_CONNECTION_INFO,
) => Promise<RecommendConstraint[]>;

getNativeSql: (options: IbisDryPlanOptions) => Promise<string>;
validate: (
dataSource: DataSourceName,
rule: ValidationRules,
Expand All @@ -123,6 +129,26 @@ export class IbisAdaptor implements IIbisAdaptor {
constructor({ ibisServerEndpoint }: { ibisServerEndpoint: string }) {
this.ibisServerBaseUrl = `${ibisServerEndpoint}/v2/connector`;
}
public async getNativeSql(options: IbisDryPlanOptions): Promise<string> {
const { dataSource, mdl, sql } = options;
const body = {
sql,
manifestStr: Buffer.from(JSON.stringify(mdl)).toString('base64'),
};
try {
const res = await axios.post(
`${this.ibisServerBaseUrl}/${dataSourceUrlMap[dataSource]}/dry-plan`,
body,
);
return res.data;
} catch (e) {
logger.debug(`Got error when dry plan with ibis: ${e.response.data}`);
throw Errors.create(Errors.GeneralErrorCodes.DRY_PLAN_ERROR, {
customMessage: e.response.data,
originalError: e,
});
}
}

public async query(
query: string,
Expand Down
5 changes: 4 additions & 1 deletion wren-ui/src/apollo/server/adaptors/wrenEngineAdaptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,10 @@ export class WrenEngineAdaptor implements IWrenEngineAdaptor {
return res.data;
} catch (err: any) {
logger.debug(`Got error when getting native SQL: ${err.message}`);
throw err;
Errors.create(Errors.GeneralErrorCodes.DRY_PLAN_ERROR, {
customMessage: err.message,
originalError: err,
});
}
}

Expand Down
30 changes: 22 additions & 8 deletions wren-ui/src/apollo/server/resolvers/modelResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import {
UpdateViewMetadataInput,
PreviewSQLData,
} from '../models';
import { IContext, RelationData, UpdateRelationData } from '../types';
import {
DataSourceName,
IContext,
RelationData,
UpdateRelationData,
} from '../types';
import { getLogger, transformInvalidColumnName } from '@server/utils';
import { DeployResponse } from '../services/deployService';
import { constructCteSql } from '../services/askingService';
Expand Down Expand Up @@ -684,8 +689,8 @@ export class ModelResolver {
const { responseId } = args;

// If using a sample dataset, native SQL is not supported
const { sampleDataset } = await ctx.projectService.getCurrentProject();
if (sampleDataset) {
const project = await ctx.projectService.getCurrentProject();
if (project.sampleDataset) {
throw new Error(`Doesn't support Native SQL`);
}
const { manifest } = await ctx.mdlService.makeCurrentModelMDL();
Expand All @@ -699,11 +704,20 @@ export class ModelResolver {
// construct cte sql and format it
const steps = response.detail.steps;
const sql = format(constructCteSql(steps));

return await ctx.wrenEngineAdaptor.getNativeSQL(sql, {
manifest,
modelingOnly: false,
});
if (project.type === DataSourceName.DUCKDB) {
logger.info(`Getting native sql from wren engine`);
return await ctx.wrenEngineAdaptor.getNativeSQL(sql, {
manifest,
modelingOnly: false,
});
} else {
logger.info(`Getting native sql from ibis server`);
return await ctx.ibisServerAdaptor.getNativeSql({
dataSource: project.type,
sql,
mdl: manifest,
});
}
}

public async updateViewMetadata(
Expand Down
3 changes: 3 additions & 0 deletions wren-ui/src/apollo/server/utils/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export enum GeneralErrorCodes {

// dry run error
DRY_RUN_ERROR = 'DRY_RUN_ERROR',
DRY_PLAN_ERROR = 'DRY_PLAN_ERROR',
}

export const errorMessages = {
Expand Down Expand Up @@ -72,6 +73,7 @@ export const errorMessages = {

// dry run error
[GeneralErrorCodes.DRY_RUN_ERROR]: 'Dry run sql statement error',
[GeneralErrorCodes.DRY_PLAN_ERROR]: 'Dry plan error',
};

export const shortMessages = {
Expand All @@ -89,6 +91,7 @@ export const shortMessages = {
[GeneralErrorCodes.INVALID_CALCULATED_FIELD]: 'Invalid calculated field',
[GeneralErrorCodes.INVALID_VIEW_CREATION]: 'Invalid view creation',
[GeneralErrorCodes.DRY_RUN_ERROR]: 'Dry run sql statement error',
[GeneralErrorCodes.DRY_PLAN_ERROR]: 'Dry plan error',
};

export const create = (
Expand Down
Loading