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 19 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
38 changes: 10 additions & 28 deletions lib/connection/statement.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,29 +397,14 @@ function createContextPreExec(
Errors.assertInternal(Util.isObject(services));
Errors.assertInternal(Util.isObject(connectionConfig));

// if we're not in qa mode, use a random uuid for the statement request id
// or use request id passed by user
if (!connectionConfig.isQaMode())
{
if (statementOptions.requestId)
{
statementContext.requestId = statementOptions.requestId;
statementContext.resubmitRequest = true;
}
else
{
statementContext.requestId = uuidv4();
}
// use request id passed by user
if (statementOptions.requestId) {
statementContext.requestId = statementOptions.requestId;
statementContext.resubmitRequest = true;
}
else // we're in qa mode
{
// if a request id or sequence id are specified in the statement options,
// use them as is; this is to facilitate testing by making things more
// deterministic
if (Util.isString(statementOptions.requestId))
{
statementContext.requestId = statementOptions.requestId;
}
else {
// use a random uuid for the statement request id
statementContext.requestId = uuidv4();
}

return statementContext;
Expand Down Expand Up @@ -1310,12 +1295,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
73 changes: 73 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,79 @@ 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=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:
{
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:
{
Expand Down
83 changes: 83 additions & 0 deletions test/unit/snowflake_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,89 @@ describe('connection.execute() statement failure', function ()
});
});

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

before(function (done) {
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');

done();
});
});

it('keep original sqlText when resubmitting requests', function (done) {
async.series(
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
[
function (callback) {
// request with sqlText and requestId specified
const statement = connection.execute(
{
sqlText: sqlText,
requestId: requestId,
complete: function (err, stmt) {
// if there's an error, fail the test with the error
if (err) {
callback(err);
}
else {
assert.ok(!err, 'there should be no error');
assert.strictEqual(stmt, statement,
'the execute() callback should be invoked with the statement');

// 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);

callback();
}
}
});
}
],
function (err) {
done(err);
});
});

it('sqlText is overwritten when resubmitting requests', function (done) {
async.series(
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
[
function (callback) {
// request with only requestId specified
const statement = connection.execute(
{
// intentionally leave sqlText blank to invoke the connector to overwrite the sqlText
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
sqlText: blankSqlText,
requestId: requestId,
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');

// the sql text and request id should be the same as what was passed
// in
assert.strictEqual(stmt.getRequestId(), requestId);
// the sqlText on the statement is unchanged but the sqlText on the request is different
assert.strictEqual(stmt.getSqlText(), blankSqlText);

callback();
}
});
}
],
function (err) {
done(err);
});
});
});

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