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

SNOW-728803: Do not replace sqlText if it is specified by the user #639

Merged
merged 22 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5b74a44
SNOW-728803: Do not replace sqlText if it is specified by the user
sfc-gh-ext-simba-lf Sep 18, 2023
e19bea1
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Sep 19, 2023
3870a87
SNOW-728803: Reformat with eslint
sfc-gh-ext-simba-lf Sep 19, 2023
c97cb40
Merge branch 'master' into SNOW-728803-request-id-sqlText
sfc-gh-ext-simba-lf Sep 21, 2023
84d6dd3
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Sep 21, 2023
2f6a4ab
SNOW-728803: Add test for special case when resubmitting requets
sfc-gh-ext-simba-lf Sep 21, 2023
b9d0eb5
Merge branch 'SNOW-728803-request-id-sqlText' of https://github.com/s…
sfc-gh-ext-simba-lf Sep 21, 2023
1f3b6c7
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Sep 22, 2023
fefa652
SNOW-728803: Modify request to include queryContextDTO property
sfc-gh-ext-simba-lf Sep 22, 2023
67d5f4c
SNOW-728803: Replace done(err) with callback(err)
sfc-gh-ext-simba-lf Sep 25, 2023
5fc404a
SNOW-728803: Wrap in else clause
sfc-gh-ext-simba-lf Sep 25, 2023
faf116c
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Sep 27, 2023
21f7044
SNOW-728803: Set requestId the same for QA and non-QA mode
sfc-gh-ext-simba-lf Sep 27, 2023
3d17073
SNOW-728803: Fix spacing of bracket
sfc-gh-ext-simba-lf Sep 27, 2023
86e9076
SNOW-728803: Add test case for sqlText being overwritten
sfc-gh-ext-simba-lf Sep 28, 2023
179acd4
Merge branch 'master' into SNOW-728803-request-id-sqlText
sfc-gh-ext-simba-lf Sep 28, 2023
a7b405b
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Sep 29, 2023
1c01521
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Sep 29, 2023
7d4b23c
SNOW-728803: Clean up tests
sfc-gh-ext-simba-lf Sep 29, 2023
b0467dc
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Oct 5, 2023
4a45148
SNOW-728803: Remove async series
sfc-gh-ext-simba-lf Oct 5, 2023
2fbd976
Merge branch 'master' into SNOW-728803-request-id-sqlText
sfc-gh-ext-simba-lf Oct 6, 2023
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
15 changes: 9 additions & 6 deletions lib/connection/statement.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,12 @@ function createContextPreExec(
{
statementContext.requestId = statementOptions.requestId;
}

// SNOW-728803: In QA mode, set resubmitRequest to true to test that sqlText
// is not overwritten if a sqlText is already specified by the user
if (statementOptions.requestId === 'SNOW-728803-requestId') {
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
statementContext.resubmitRequest = true;
}
}

return statementContext;
Expand Down Expand Up @@ -1310,12 +1316,9 @@ function sendRequestPreExec(statementContext, onResultAvailable)
{
disableOfflineChunks: false,
};
if (!statementContext.resubmitRequest)
{
json.sqlText = statementContext.sqlText;
}
else
{
json.sqlText = statementContext.sqlText;

if (statementContext.resubmitRequest && !json.sqlText) {
json.sqlText = `SELECT 'Error retrieving query results for request id: ${statementContext.requestId}, `
+ `please use RESULT_SCAN instead' AS ErrorMessage;`;
}
Expand Down
108 changes: 108 additions & 0 deletions test/unit/mock/mock_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,114 @@ function buildRequestOutputMappings(clientInfo)
}
}
},
{
request:
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
{
method: 'POST',
url: 'http://fakeaccount.snowflakecomputing.com/queries/v1/query-request?requestId=',
headers:
{
'Accept': 'application/snowflake',
'Authorization': 'Snowflake Token="SESSION_TOKEN"',
'Content-Type': 'application/json'
},
json:
{
disableOfflineChunks: false,
sqlText: 'select 1;',
queryContextDTO: { entries: [] }
}
},
output:
{
err: null,
response:
{
statusCode: 401,
statusMessage: 'Unauthorized',
body:
{
'data' : null,
'code': '390103',
'message': 'Session token not found in the request data.',
'success': false,
}
}
}
},
{
request:
{
method: 'POST',
url: 'http://fakeaccount.snowflakecomputing.com/queries/v1/query-request?requestId=SNOW-728803-requestId',
headers:
{
'Accept': 'application/snowflake',
'Authorization': 'Snowflake Token="SESSION_TOKEN"',
'Content-Type': 'application/json'
},
json:
{
disableOfflineChunks: false,
sqlText: 'SELECT \'Error retrieving query results for request id: SNOW-728803-requestId, please use RESULT_SCAN instead\' AS ErrorMessage;',
queryContextDTO: { entries: [] }
}
},
output:
{
err: null,
response:
{
body:
{
'message': 'The specified sqlText should not be overwritten when resubmitting the request',
'success': false
}
}
}
},
{
request:
{
method: 'POST',
url: 'http://fakeaccount.snowflakecomputing.com/queries/v1/query-request?requestId=SNOW-728803-requestId',
headers:
{
'Accept': 'application/snowflake',
'Authorization': 'Snowflake Token="SESSION_TOKEN"',
'Content-Type': 'application/json'
},
json:
{
disableOfflineChunks: false,
sqlText: 'select 1;',
queryContextDTO: { entries: [] }
}
},
output:
{
err: null,
response:
{
statusCode: 200,
statusMessage: 'OK',
body:
{
'data':
{
'parameters': [],
'rowtype': [],
'rowset': [['1']],
'total': 1,
'returned': 1
},
'message': null,
'code': null,
'success': true
}
}
}
},
{
request:
{
Expand Down
65 changes: 65 additions & 0 deletions test/unit/snowflake_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,71 @@ describe('connection.execute() statement failure', function ()
});
});

describe('connection.execute() with requestId', function () {
const connection = snowflake.createConnection(connectionOptions);
const sqlText = 'select 1;';
const requestId = 'SNOW-728803-requestId';

it('keep original sqlText when resubmitting requests', function (done) {
let statement;

async.series(
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
[
function (callback) {
connection.connect(function (err, conn) {
assert.ok(!err, 'there should be no error');
assert.strictEqual(conn, connection,
'the connect() callback should be invoked with the statement');

callback();
});
},
function (callback) {
// 1st request fails with an error
statement = connection.execute(
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
{
sqlText: sqlText,
complete: function (err, stmt) {
assert.ok(err, 'there should be an error');
assert.strictEqual(stmt, statement,
'the execute() callback should be invoked with the statement');

callback();
}
});
},
function (callback) {
// 2nd request with sqlText and requestId specified
statement = connection.execute(
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
{
sqlText: sqlText,
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
requestId: requestId,
complete: function (err, stmt) {
// if there's an error, fail the test with the error
if (err) {
done(err);
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
}
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

assert.ok(!err, 'there should be no error');
assert.strictEqual(stmt, statement,
'the execute() callback should be invoked with the statement');

callback();
}
});

// the sql text and request id should be the same as what was passed
// in
assert.strictEqual(statement.getSqlText(), sqlText);
assert.strictEqual(statement.getRequestId(), requestId);
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
}
],
function () {
done();
});
});
});

describe('too many concurrent requests', function ()
{
it('too many concurrent requests per user', function (done)
Expand Down
Loading