Skip to content

Commit

Permalink
fix(FIR-37001): fix parameter formatting for new FB version (#119)
Browse files Browse the repository at this point in the history
  • Loading branch information
ptiurin authored Oct 2, 2024
1 parent 6a1b223 commit 9e383ae
Show file tree
Hide file tree
Showing 16 changed files with 520 additions and 80 deletions.
17 changes: 12 additions & 5 deletions src/connection/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Statement } from "../statement";
import { generateUserAgent } from "../common/util";
import { CompositeError } from "../common/errors";
import JSONbig from "json-bigint";
import { QueryFormatter } from "../formatter/base";

const defaultQuerySettings = {
output_format: OutputFormat.COMPACT
Expand All @@ -26,15 +27,21 @@ const testConnectionQuery = "SELECT 1";

export abstract class Connection {
protected context: Context;
protected queryFormatter: QueryFormatter;
protected options: ConnectionOptions;
protected userAgent: string;
protected parameters: Record<string, string>;
engineEndpoint!: string;
activeRequests = new Set<{ abort: () => void }>();

constructor(context: Context, options: ConnectionOptions) {
constructor(
queryFormatter: QueryFormatter,
context: Context,
options: ConnectionOptions
) {
this.context = context;
this.options = options;
this.queryFormatter = queryFormatter;
this.parameters = {
...(options.database ? { database: options.database } : {}),
...defaultQuerySettings
Expand Down Expand Up @@ -147,7 +154,7 @@ export abstract class Connection {
query: string,
executeQueryOptions: ExecuteQueryOptions = {}
): Promise<Statement> {
const { httpClient, queryFormatter } = this.context;
const { httpClient } = this.context;

executeQueryOptions.response = {
...defaultResponseSettings,
Expand All @@ -159,12 +166,12 @@ export abstract class Connection {
let setKey = "",
setValue = "",
formattedQuery: string;
if (queryFormatter.isSetStatement(query)) {
[setKey, setValue] = queryFormatter.splitSetStatement(query);
if (this.queryFormatter.isSetStatement(query)) {
[setKey, setValue] = this.queryFormatter.splitSetStatement(query);
this.parameters[setKey] = setValue;
formattedQuery = testConnectionQuery;
} else {
formattedQuery = queryFormatter.formatQuery(
formattedQuery = this.queryFormatter.formatQuery(
query,
parameters,
namedParameters
Expand Down
8 changes: 6 additions & 2 deletions src/connection/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
} from "../types";
import { ConnectionV1 } from "./connection_v1";
import { ConnectionV2 } from "./connection_v2";
import { QueryFormatterV1 } from "../formatter/formatter_v1";
import { QueryFormatterV2 } from "../formatter/formatter_v2";

export type { Connection } from "./base";

Expand All @@ -14,12 +16,14 @@ export function makeConnection(context: Context, options: ConnectionOptions) {
(options.auth as ServiceAccountAuth).client_id &&
(options.auth as ServiceAccountAuth).client_secret
) {
return new ConnectionV2(context, options);
const queryFormatter = new QueryFormatterV2();
return new ConnectionV2(queryFormatter, context, options);
} else if (
(options.auth as UsernamePasswordAuth).username &&
(options.auth as UsernamePasswordAuth).password
) {
return new ConnectionV1(context, options);
const queryFormatter = new QueryFormatterV1();
return new ConnectionV1(queryFormatter, context, options);
}
throw new Error("Invalid auth options");
}
6 changes: 1 addition & 5 deletions src/firebolt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Logger } from "./logger";
import { HttpClient } from "./http";
import { ResourceManager } from "./service";
import { FireboltCore } from "./core";
import { QueryFormatter } from "./formatter";
import { FireboltClientOptions, ResourceManagerOptions } from "./types";

type Dependencies = {
Expand Down Expand Up @@ -34,13 +33,10 @@ const getContext = (
const httpClient =
options.dependencies?.httpClient || new DefaultHttpClient(clientOptions);

const queryFormatter = new QueryFormatter();

const context = {
logger,
httpClient,
apiEndpoint,
queryFormatter
apiEndpoint
};
return context;
};
Expand Down
40 changes: 10 additions & 30 deletions src/formatter/index.ts → src/formatter/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,6 @@ import BigNumber from "bignumber.js";
import { checkArgumentValid, zeroPad } from "../common/util";
import { INVALID_PARAMETERS } from "../common/errors";

const CHARS_GLOBAL_REGEXP = /[\0\b\t\n\r\x1a"'\\]/g; // eslint-disable-line no-control-regex

const CHARS_ESCAPE_MAP: Record<string, string> = {
"\0": "\\0",
"\b": "\\b",
"\t": "\\t",
"\n": "\\n",
"\r": "\\r",
"\x1a": "\\Z",
'"': '\\"',
"'": "\\'",
"\\": "\\\\"
};
const SET_PREFIX = "set ";

export class Tuple {
Expand All @@ -38,27 +25,20 @@ export class TimestampTZ extends Date {

export class TimestampNTZ extends Date {}

export class QueryFormatter {
export abstract class QueryFormatter {
abstract get CHARS_GLOBAL_REGEXP(): RegExp;
abstract get CHARS_ESCAPE_MAP(): Record<string, string>;
abstract get tokenizer(): RegExp;

private format(
query: string,
params: unknown[],
namedParams: Record<string, unknown>
) {
params = [...params];

// Matches:
// - ' strings with \ escapes
// - " strings with \ escapes
// - /* */ comments
// - -- comments
// - ? parameters
// - :: operator
// - :named parameters
const tokenizer =
/'(?:[^'\\]+|\\.)*'|"(?:[^"\\]+|\\.)*"|\/\*[\s\S]*\*\/|--.*|(\?)|::|:(\w+)/g;

query = query.replace(
tokenizer,
this.tokenizer,
(str, param: string | undefined, paramName: string | undefined) => {
if (param) {
if (params.length == 0) {
Expand Down Expand Up @@ -113,15 +93,15 @@ export class QueryFormatter {
}

private escapeString(param: string) {
let chunkIndex = (CHARS_GLOBAL_REGEXP.lastIndex = 0);
let chunkIndex = (this.CHARS_GLOBAL_REGEXP.lastIndex = 0);
let escapedValue = "";
let match;

while ((match = CHARS_GLOBAL_REGEXP.exec(param))) {
while ((match = this.CHARS_GLOBAL_REGEXP.exec(param))) {
const key = match[0];
escapedValue +=
param.slice(chunkIndex, match.index) + CHARS_ESCAPE_MAP[key];
chunkIndex = CHARS_GLOBAL_REGEXP.lastIndex;
param.slice(chunkIndex, match.index) + this.CHARS_ESCAPE_MAP[key];
chunkIndex = this.CHARS_GLOBAL_REGEXP.lastIndex;
}

if (chunkIndex === 0) {
Expand Down
42 changes: 42 additions & 0 deletions src/formatter/formatter_v1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { QueryFormatter } from "./base";

const CHARS_GLOBAL_REGEXP = /[\0\b\t\n\r\x1a"'\\]/g; // eslint-disable-line no-control-regex

const CHARS_ESCAPE_MAP: Record<string, string> = {
"\0": "\\0",
"\b": "\\b",
"\t": "\\t",
"\n": "\\n",
"\r": "\\r",
"\x1a": "\\Z",
'"': '\\"',
"'": "\\'",
"\\": "\\\\"
};
// Matches:
// - ' strings with \ escapes
// - " strings with \ escapes
// - /* */ comments
// - -- comments
// - ? parameters
// - :: operator
// - :named parameters
// Captures the ? and :named parameters that are not inside strings or quotes
// and special characters
const tokenizer =
/'(?:[^'\\]+|\\.)*'|"(?:[^"\\]+|\\.)*"|\/\*[\s\S]*\*\/|--.*|(\?)|::|:(\w+)/g;

export class QueryFormatterV1 extends QueryFormatter {
constructor() {
super();
}
get CHARS_GLOBAL_REGEXP() {
return CHARS_GLOBAL_REGEXP;
}
get CHARS_ESCAPE_MAP() {
return CHARS_ESCAPE_MAP;
}
get tokenizer() {
return tokenizer;
}
}
33 changes: 33 additions & 0 deletions src/formatter/formatter_v2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { QueryFormatter } from "./base";

const CHARS_GLOBAL_REGEXP = /[']/g; // eslint-disable-line no-control-regex

const CHARS_ESCAPE_MAP: Record<string, string> = {
"'": "''"
};
// Matches:
// - ' strings
// - " strings
// - /* */ comments
// - -- comments
// - ? parameters
// - :: operator
// - :named parameters
// Captures the ? and :named parameters that are not inside strings or quotes
// and quotes that need escaping
const tokenizer = /'[^']*'|"[^"]*"|\/\*[\s\S]*\*\/|--.*|(\?)|::|:(\w+)/g;

export class QueryFormatterV2 extends QueryFormatter {
constructor() {
super();
}
get CHARS_GLOBAL_REGEXP() {
return CHARS_GLOBAL_REGEXP;
}
get CHARS_ESCAPE_MAP() {
return CHARS_ESCAPE_MAP;
}
get tokenizer() {
return tokenizer;
}
}
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export {
TimestampTZ,
TimestampNTZ,
QueryFormatter
} from "./formatter";
} from "./formatter/base";

export { JSONParser } from "./statement/stream/parser";

Expand Down
2 changes: 0 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { HttpClientInterface, HttpClientOptions } from "./http";

import { LoggerInterface, LoggerOptions } from "./logger";
import { Meta } from "./meta";
import { QueryFormatter } from "./formatter";
import { Connection } from "./connection";
import { ConnectionV1 } from "./connection/connection_v1";

Expand Down Expand Up @@ -109,7 +108,6 @@ export type ResourceManagerOptions = FireboltClientOptions & {
export type Context = {
logger: LoggerInterface;
httpClient: HttpClientInterface;
queryFormatter: QueryFormatter;
apiEndpoint: string;
};

Expand Down
23 changes: 23 additions & 0 deletions test/integration/v1/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,29 @@ describe("integration test", () => {
expect(value).toBeInstanceOf(Date);
}
});
it("string quoting", async () => {
const firebolt = Firebolt({
apiEndpoint: process.env.FIREBOLT_API_ENDPOINT as string
});

const connection = await firebolt.connect(connectionParams);
const statement = await connection.execute("select ? as json", {
response: { normalizeData: true },
parameters: [`{"key":"val"}`]
});
const { data } = await statement.fetchResult();
const row_parameterised = data[0];
const statement2 = await connection.execute(
`select '{"key":"val"}' as json`,
{
response: { normalizeData: true }
}
);
const { data: data2 } = await statement2.fetchResult();
const row_literal = data2[0];
expect(row_parameterised).toMatchObject(row_literal);
expect(row_parameterised).toMatchObject({ json: `{"key":"val"}` });
});
it("fails on no engine found", async () => {
const firebolt = Firebolt({
apiEndpoint: process.env.FIREBOLT_API_ENDPOINT as string
Expand Down
26 changes: 24 additions & 2 deletions test/integration/v2/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,29 @@ describe("integration test", () => {
expect(value).toBeInstanceOf(Date);
}
});
it("string quoting", async () => {
const firebolt = Firebolt({
apiEndpoint: process.env.FIREBOLT_API_ENDPOINT as string
});

const connection = await firebolt.connect(connectionParams);
const statement = await connection.execute("select ? as json", {
response: { normalizeData: true },
parameters: [`{"key":"val"}`]
});
const { data } = await statement.fetchResult();
const row_parameterised = data[0];
const statement2 = await connection.execute(
`select '{"key":"val"}' as json`,
{
response: { normalizeData: true }
}
);
const { data: data2 } = await statement2.fetchResult();
const row_literal = data2[0];
expect(row_parameterised).toMatchObject(row_literal);
expect(row_parameterised).toMatchObject({ json: `{"key":"val"}` });
});
it("fails on no engine found", async () => {
const firebolt = Firebolt({
apiEndpoint: process.env.FIREBOLT_API_ENDPOINT as string
Expand Down Expand Up @@ -268,8 +291,7 @@ describe("integration test", () => {
await firebolt.testConnection(connectionParams);
expect(true).toBeTruthy();
});
// Since streaming is currently disabled, custom parser is not supported
it.skip("custom parser", async () => {
it("custom parser", async () => {
const firebolt = Firebolt({
apiEndpoint: process.env.FIREBOLT_API_ENDPOINT as string
});
Expand Down
14 changes: 9 additions & 5 deletions test/unit/connection.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { setupServer } from "msw/node";
import { rest } from "msw";
import { Firebolt, QueryFormatter } from "../../src";
import { Firebolt } from "../../src";
import { QueryFormatterV2 as QueryFormatter } from "../../src/formatter/formatter_v2";
import { ConnectionOptions } from "../../src/types";
import { QUERY_URL } from "../../src/common/api";
import { ConnectionV2 } from "../../src/connection/connection_v2";
Expand Down Expand Up @@ -433,12 +434,15 @@ INFO: SYNTAX_ERROR - Unexpected character at {"failingLine":42,"startOffset":120
const context = {
logger: new Logger(),
httpClient: new NodeHttpClient(),
apiEndpoint,
queryFormatter: new QueryFormatter()
apiEndpoint
};

const queryFormatter = new QueryFormatter();
const auth = new Authenticator(context, connectionOptions);
const connection = new MockConnection(context, connectionOptions);
const connection = new MockConnection(
queryFormatter,
context,
connectionOptions
);
await auth.authenticate();
await connection.resolveEngineEndpoint();
return connection;
Expand Down
Loading

0 comments on commit 9e383ae

Please sign in to comment.