Skip to content

Commit 6739115

Browse files
authored
disable compression if cloud fetch is enabled (#297)
1 parent c085195 commit 6739115

File tree

4 files changed

+93
-12
lines changed

4 files changed

+93
-12
lines changed

lib/DBSQLSession.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,14 @@ export default class DBSQLSession implements IDBSQLSession {
227227
request.parameters = getQueryParameters(options.namedParameters, options.ordinalParameters);
228228
}
229229

230-
if (ProtocolVersion.supportsArrowCompression(this.serverProtocolVersion)) {
231-
request.canDecompressLZ4Result = (options.useLZ4Compression ?? clientConfig.useLZ4Compression) && Boolean(LZ4);
232-
}
233-
234230
if (ProtocolVersion.supportsCloudFetch(this.serverProtocolVersion)) {
235231
request.canDownloadResult = options.useCloudFetch ?? clientConfig.useCloudFetch;
236232
}
237233

234+
if (ProtocolVersion.supportsArrowCompression(this.serverProtocolVersion) && request.canDownloadResult !== true) {
235+
request.canDecompressLZ4Result = (options.useLZ4Compression ?? clientConfig.useLZ4Compression) && Boolean(LZ4);
236+
}
237+
238238
const operationPromise = driver.executeStatement(request);
239239
const response = await this.handleResponse(operationPromise);
240240
const operation = this.createOperation(response);

tests/e2e/arrow.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,10 @@ describe('Arrow support', () => {
187187
'should handle LZ4 compressed data',
188188
createTest(
189189
async (session) => {
190-
const operation = await session.executeStatement(`SELECT * FROM ${tableName}`);
190+
const operation = await session.executeStatement(
191+
`SELECT * FROM ${tableName}`,
192+
{ useCloudFetch: false }, // Explicitly disable cloud fetch to test LZ4 compression
193+
);
191194
const result = await operation.fetchAll();
192195
expect(fixArrowResult(result)).to.deep.equal(expectedArrow);
193196

tests/e2e/cloudfetch.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ describe('CloudFetch', () => {
9797
expect(fetchedRowCount).to.be.equal(queriedRowsCount);
9898
});
9999

100-
it('should handle LZ4 compressed data', async () => {
100+
it('should not use LZ4 compression with cloud fetch', async () => {
101101
const cloudFetchConcurrentDownloads = 5;
102102
const session = await openSession({
103103
cloudFetchConcurrentDownloads,
104-
useLZ4Compression: true,
104+
useLZ4Compression: true, // This is ignored when cloud fetch is enabled
105105
});
106106

107107
const queriedRowsCount = 10000000; // result has to be quite big to enable CloudFetch
@@ -126,7 +126,8 @@ describe('CloudFetch', () => {
126126
expect(resultHandler).to.be.instanceof(ResultSlicer);
127127
expect(resultHandler.source).to.be.instanceof(ArrowResultConverter);
128128
expect(resultHandler.source.source).to.be.instanceOf(CloudFetchResultHandler);
129-
expect(resultHandler.source.source.isLZ4Compressed).to.be.true;
129+
// LZ4 compression should not be enabled with cloud fetch
130+
expect(resultHandler.source.source.isLZ4Compressed).to.be.false;
130131

131132
const chunk = await operation.fetchChunk({ maxRows: 100000, disableBuffering: true });
132133
expect(chunk.length).to.be.gt(0);

tests/unit/DBSQLSession.test.ts

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ describe('DBSQLSession', () => {
8686
});
8787

8888
it('should apply defaults for Arrow options', async () => {
89-
case1: {
89+
// case 1
90+
{
9091
const session = new DBSQLSession({
9192
handle: sessionHandleStub,
9293
context: new ClientContextStub({ arrowEnabled: true }),
@@ -95,7 +96,8 @@ describe('DBSQLSession', () => {
9596
expect(result).instanceOf(DBSQLOperation);
9697
}
9798

98-
case2: {
99+
// case 2
100+
{
99101
const session = new DBSQLSession({
100102
handle: sessionHandleStub,
101103
context: new ClientContextStub({ arrowEnabled: true, useArrowNativeTypes: false }),
@@ -158,9 +160,14 @@ describe('DBSQLSession', () => {
158160
}
159161

160162
if (version >= TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V6) {
161-
expect(req.canDecompressLZ4Result).to.be.true;
163+
// Since cloud fetch is enabled, canDecompressLZ4Result should not be set
164+
if (req.canDownloadResult === true) {
165+
expect(req.canDecompressLZ4Result).to.not.be.true;
166+
} else {
167+
expect(req.canDecompressLZ4Result).to.be.true;
168+
}
162169
} else {
163-
expect(req.canDecompressLZ4Result).to.not.exist;
170+
expect(req.canDecompressLZ4Result).to.not.be.true;
164171
}
165172

166173
if (version >= TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V5) {
@@ -180,6 +187,76 @@ describe('DBSQLSession', () => {
180187
});
181188
}
182189
});
190+
191+
describe('LZ4 compression with cloud fetch', () => {
192+
it('should not set canDecompressLZ4Result when cloud fetch is enabled (canDownloadResult=true)', async () => {
193+
const context = new ClientContextStub({ useLZ4Compression: true });
194+
const driver = sinon.spy(context.driver);
195+
const statement = 'SELECT * FROM table';
196+
197+
// Use V6+ which supports arrow compression
198+
const session = new DBSQLSession({
199+
handle: sessionHandleStub,
200+
context,
201+
serverProtocolVersion: TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V6,
202+
});
203+
204+
// Execute with cloud fetch enabled
205+
await session.executeStatement(statement, { useCloudFetch: true });
206+
207+
expect(driver.executeStatement.callCount).to.eq(1);
208+
const req = driver.executeStatement.firstCall.args[0];
209+
210+
// canDownloadResult should be true and canDecompressLZ4Result should NOT be set
211+
expect(req.canDownloadResult).to.be.true;
212+
expect(req.canDecompressLZ4Result).to.not.be.true;
213+
});
214+
215+
it('should set canDecompressLZ4Result when cloud fetch is disabled (canDownloadResult=false)', async () => {
216+
const context = new ClientContextStub({ useLZ4Compression: true });
217+
const driver = sinon.spy(context.driver);
218+
const statement = 'SELECT * FROM table';
219+
220+
// Use V6+ which supports arrow compression
221+
const session = new DBSQLSession({
222+
handle: sessionHandleStub,
223+
context,
224+
serverProtocolVersion: TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V6,
225+
});
226+
227+
// Execute with cloud fetch disabled
228+
await session.executeStatement(statement, { useCloudFetch: false });
229+
230+
expect(driver.executeStatement.callCount).to.eq(1);
231+
const req = driver.executeStatement.firstCall.args[0];
232+
233+
// canDownloadResult should be false and canDecompressLZ4Result should be set
234+
expect(req.canDownloadResult).to.be.false;
235+
expect(req.canDecompressLZ4Result).to.be.true;
236+
});
237+
238+
it('should not set canDecompressLZ4Result when server protocol does not support Arrow compression', async () => {
239+
const context = new ClientContextStub({ useLZ4Compression: true });
240+
const driver = sinon.spy(context.driver);
241+
const statement = 'SELECT * FROM table';
242+
243+
// Use V5 which does not support arrow compression
244+
const session = new DBSQLSession({
245+
handle: sessionHandleStub,
246+
context,
247+
serverProtocolVersion: TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V5,
248+
});
249+
250+
// Execute with cloud fetch disabled
251+
await session.executeStatement(statement, { useCloudFetch: false });
252+
253+
expect(driver.executeStatement.callCount).to.eq(1);
254+
const req = driver.executeStatement.firstCall.args[0];
255+
256+
// canDecompressLZ4Result should NOT be set regardless of cloud fetch setting
257+
expect(req.canDecompressLZ4Result).to.not.be.true;
258+
});
259+
});
183260
});
184261

185262
describe('getTypeInfo', () => {

0 commit comments

Comments
 (0)