Skip to content

Commit b74dade

Browse files
nithinkdbkravets-levko
nithinkdb
and
kravets-levko
authored
[PECO-1056] Add support for ordinal parameters (#182)
* Added changes and test * Fixed version check * Fixed test * Linted * Refine code, add more tests Signed-off-by: Levko Kravets <levko.ne@gmail.com> --------- Signed-off-by: Levko Kravets <levko.ne@gmail.com> Co-authored-by: Levko Kravets <levko.ne@gmail.com>
1 parent e69ee4b commit b74dade

4 files changed

Lines changed: 145 additions & 24 deletions

File tree

‎lib/DBSQLSession.ts‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import HiveDriverError from './errors/HiveDriverError';
3737
import globalConfig from './globalConfig';
3838
import StagingError from './errors/StagingError';
3939
import { DBSQLParameter, DBSQLParameterValue } from './DBSQLParameter';
40+
import ParameterError from './errors/ParameterError';
4041

4142
const defaultMaxRows = 100000;
4243

@@ -85,26 +86,46 @@ function getArrowOptions(): {
8586
function getQueryParameters(
8687
sessionHandle: TSessionHandle,
8788
namedParameters?: Record<string, DBSQLParameter | DBSQLParameterValue>,
89+
ordinalParameters?: Array<DBSQLParameter | DBSQLParameterValue>,
8890
): Array<TSparkParameter> {
91+
const namedParametersProvided = namedParameters !== undefined && Object.keys(namedParameters).length > 0;
92+
const ordinalParametersProvided = ordinalParameters !== undefined && ordinalParameters.length > 0;
93+
94+
if (namedParametersProvided && ordinalParametersProvided) {
95+
throw new ParameterError('Driver does not support both ordinal and named parameters.');
96+
}
97+
98+
if (!namedParametersProvided && !ordinalParametersProvided) {
99+
return [];
100+
}
101+
102+
if (
103+
!sessionHandle.serverProtocolVersion ||
104+
sessionHandle.serverProtocolVersion < TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V8
105+
) {
106+
throw new Thrift.TProtocolException(
107+
Thrift.TProtocolExceptionType.BAD_VERSION,
108+
'Server version does not support parameterized queries',
109+
);
110+
}
111+
89112
const result: Array<TSparkParameter> = [];
90113

91114
if (namedParameters !== undefined) {
92-
if (
93-
sessionHandle?.serverProtocolVersion &&
94-
sessionHandle.serverProtocolVersion >= TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V8
95-
) {
96-
for (const name of Object.keys(namedParameters)) {
97-
const value = namedParameters[name];
98-
const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value });
99-
const sparkParam = param.toSparkParameter();
100-
sparkParam.name = name;
101-
result.push(sparkParam);
102-
}
103-
} else {
104-
throw new Thrift.TProtocolException(
105-
Thrift.TProtocolExceptionType.BAD_VERSION,
106-
'Server version does not support parameterized queries',
107-
);
115+
for (const name of Object.keys(namedParameters)) {
116+
const value = namedParameters[name];
117+
const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value });
118+
const sparkParam = param.toSparkParameter();
119+
sparkParam.name = name;
120+
result.push(sparkParam);
121+
}
122+
}
123+
124+
if (ordinalParameters !== undefined) {
125+
for (const value of ordinalParameters) {
126+
const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value });
127+
const sparkParam = param.toSparkParameter();
128+
result.push(sparkParam);
108129
}
109130
}
110131

@@ -177,7 +198,7 @@ export default class DBSQLSession implements IDBSQLSession {
177198
...getDirectResultsOptions(options.maxRows),
178199
...getArrowOptions(),
179200
canDownloadResult: options.useCloudFetch ?? globalConfig.useCloudFetch,
180-
parameters: getQueryParameters(this.sessionHandle, options.namedParameters),
201+
parameters: getQueryParameters(this.sessionHandle, options.namedParameters, options.ordinalParameters),
181202
});
182203
const response = await this.handleResponse(operationPromise);
183204
const operation = this.createOperation(response);

‎lib/contracts/IDBSQLSession.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type ExecuteStatementOptions = {
1414
useCloudFetch?: boolean;
1515
stagingAllowedLocalPath?: string | string[];
1616
namedParameters?: Record<string, DBSQLParameter | DBSQLParameterValue>;
17+
ordinalParameters?: Array<DBSQLParameter | DBSQLParameterValue>;
1718
};
1819

1920
export type TypeInfoRequest = {

‎lib/errors/ParameterError.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default class ParameterError extends Error {}

‎tests/e2e/query_parameters.test.js‎

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
const { expect } = require('chai');
1+
const { expect, AssertionError } = require('chai');
22
const Int64 = require('node-int64');
33
const config = require('./utils/config');
44
const { DBSQLClient, DBSQLParameter, DBSQLParameterType } = require('../..');
5+
const ParameterError = require('../../dist/errors/ParameterError').default;
56

67
const openSession = async () => {
78
const client = new DBSQLClient();
@@ -19,8 +20,8 @@ const openSession = async () => {
1920
};
2021

2122
// TODO: Temporarily disable those tests until we figure out issues with E2E test env
22-
describe.skip('Query parameters', () => {
23-
it('should use named parameters', async () => {
23+
describe('Query parameters', () => {
24+
it.skip('should use named parameters', async () => {
2425
const session = await openSession();
2526
const operation = await session.executeStatement(
2627
`
@@ -30,8 +31,8 @@ describe.skip('Query parameters', () => {
3031
:p_double AS col_double,
3132
:p_bigint_1 AS col_bigint_1,
3233
:p_bigint_2 AS col_bigint_2,
33-
:p_date as col_date,
34-
:p_timestamp as col_timestamp,
34+
:p_date AS col_date,
35+
:p_timestamp AS col_timestamp,
3536
:p_str AS col_str
3637
`,
3738
{
@@ -62,7 +63,7 @@ describe.skip('Query parameters', () => {
6263
]);
6364
});
6465

65-
it('should accept primitives as values for named parameters', async () => {
66+
it.skip('should accept primitives as values for named parameters', async () => {
6667
const session = await openSession();
6768
const operation = await session.executeStatement(
6869
`
@@ -72,7 +73,7 @@ describe.skip('Query parameters', () => {
7273
:p_double AS col_double,
7374
:p_bigint_1 AS col_bigint_1,
7475
:p_bigint_2 AS col_bigint_2,
75-
:p_timestamp as col_timestamp,
76+
:p_timestamp AS col_timestamp,
7677
:p_str AS col_str
7778
`,
7879
{
@@ -100,4 +101,101 @@ describe.skip('Query parameters', () => {
100101
},
101102
]);
102103
});
104+
105+
it.skip('should use ordinal parameters', async () => {
106+
const session = await openSession();
107+
const operation = await session.executeStatement(
108+
`
109+
SELECT
110+
? AS col_bool,
111+
? AS col_int,
112+
? AS col_double,
113+
? AS col_bigint_1,
114+
? AS col_bigint_2,
115+
? AS col_date,
116+
? AS col_timestamp,
117+
? AS col_str
118+
`,
119+
{
120+
namedParameters: {
121+
p_bool: new DBSQLParameter({ value: true }),
122+
p_int: new DBSQLParameter({ value: 1234 }),
123+
p_double: new DBSQLParameter({ value: 3.14 }),
124+
p_bigint_1: new DBSQLParameter({ value: BigInt(1234) }),
125+
p_bigint_2: new DBSQLParameter({ value: new Int64(1234) }),
126+
p_date: new DBSQLParameter({ value: new Date('2023-09-06T03:14:27.843Z'), type: DBSQLParameterType.DATE }),
127+
p_timestamp: new DBSQLParameter({ value: new Date('2023-09-06T03:14:27.843Z') }),
128+
p_str: new DBSQLParameter({ value: 'Hello' }),
129+
},
130+
},
131+
);
132+
const result = await operation.fetchAll();
133+
expect(result).to.deep.equal([
134+
{
135+
col_bool: true,
136+
col_int: 1234,
137+
col_double: 3.14,
138+
col_bigint_1: 1234,
139+
col_bigint_2: 1234,
140+
col_date: new Date('2023-09-06T00:00:00.000Z'),
141+
col_timestamp: new Date('2023-09-06T03:14:27.843Z'),
142+
col_str: 'Hello',
143+
},
144+
]);
145+
});
146+
147+
it.skip('should accept primitives as values for ordinal parameters', async () => {
148+
const session = await openSession();
149+
const operation = await session.executeStatement(
150+
`
151+
SELECT
152+
? AS col_bool,
153+
? AS col_int,
154+
? AS col_double,
155+
? AS col_bigint_1,
156+
? AS col_bigint_2,
157+
? AS col_timestamp,
158+
? AS col_str
159+
`,
160+
{
161+
ordinalParameters: [
162+
true,
163+
1234,
164+
3.14,
165+
BigInt(1234),
166+
new Int64(1234),
167+
new Date('2023-09-06T03:14:27.843Z'),
168+
'Hello',
169+
],
170+
},
171+
);
172+
const result = await operation.fetchAll();
173+
expect(result).to.deep.equal([
174+
{
175+
col_bool: true,
176+
col_int: 1234,
177+
col_double: 3.14,
178+
col_bigint_1: 1234,
179+
col_bigint_2: 1234,
180+
col_timestamp: new Date('2023-09-06T03:14:27.843Z'),
181+
col_str: 'Hello',
182+
},
183+
]);
184+
});
185+
186+
it('should fail if both named and ordinal parameters used', async () => {
187+
const session = await openSession();
188+
189+
try {
190+
await session.executeStatement(`SELECT :p, ?`, {
191+
namedParameters: { p: 1234 },
192+
ordinalParameters: ['test'],
193+
});
194+
} catch (error) {
195+
if (error instanceof AssertionError) {
196+
throw error;
197+
}
198+
expect(error).to.be.instanceof(ParameterError);
199+
}
200+
});
103201
});

0 commit comments

Comments
 (0)