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

) * SNOW-728803: Do not replace sqlText if it is specified by the user * SNOW-728803: Reformat with eslint * SNOW-728803: Add test for special case when resubmitting requets * SNOW-728803: Modify request to include queryContextDTO property * SNOW-728803: Replace done(err) with callback(err) * SNOW-728803: Wrap in else clause * SNOW-728803: Set requestId the same for QA and non-QA mode * SNOW-728803: Fix spacing of bracket * SNOW-728803: Add test case for sqlText being overwritten * SNOW-728803: Clean up tests * SNOW-728803: Remove async series
snowflakedb · Oct 6, 2023 · 1bae9ce · 1bae9ce
1 parent d01c4f4
commit 1bae9ce
Show file tree

Hide file tree

Showing 3 changed files with 150 additions and 28 deletions.
diff --git a/lib/connection/statement.js b/lib/connection/statement.js
@@ -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;
@@ -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;`;
   }

diff --git a/test/unit/mock/mock_http_client.js b/test/unit/mock/mock_http_client.js
@@ -700,6 +700,79 @@ function buildRequestOutputMappings(clientInfo)
             }
         }
     },
+    {
+      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:
+        {
+          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:
         {

diff --git a/test/unit/snowflake_test.js b/test/unit/snowflake_test.js
@@ -936,6 +936,73 @@ 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) {
+    // 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) {
+            done(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);
+
+            done();
+          }
+        }
+      });
+  });
+
+  it('sqlText is overwritten when resubmitting requests', function (done) {
+    // request with only requestId specified
+    const statement = connection.execute(
+      {
+        // intentionally leave sqlText blank to invoke the connector to overwrite the sqlText
+        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);
+
+          done();
+        }
+      });
+  });
+});
+
 describe('too many concurrent requests', function ()
 {
   it('too many concurrent requests per user', function (done)