Fixup Statement.execute query options (#278)

Add execution timeout code path for Statement.execute
Add README.md entries for using query options with Statement.execute
Add tests to ensure setting query timeout at this point won't crash ODBC

Signed-off-by: Mark Irish <[email protected]>

Author: markirish

README.md

@@ -962,11 +962,16 @@ odbc.connect(`${process.env.CONNECTION_STRING}`, (error, connection) => {
 
 ---
 
-### `.execute(callback?)`
+### `.execute(options?, callback?)`
 
 Executes the prepared and optionally bound SQL statement.
 
 #### Parameters:
+* **options?**: An object containing options that affect execution behavior. Valid properties include:
+    * `cursor`: A boolean value indicating whether or not to return a cursor instead of results immediately. Can also be a string naming the cursor, which will assume that a cursor will be returned. Closing the `Statement` will also close the `Cursor`, but closing the `Cursor` will keep the `Statement` valid.
+    * `fetchSize`: Used with a cursor, sets the number of rows that are returned on a call to `fetch` on the Cursor.
+    * `timeout`: The amount of time (in seconds) that the query will attempt to execute before returning to the application.
+    * `initialBufferSize`: Sets the initial buffer size (in bytes) for storing data from SQL_LONG* data fields. Useful for avoiding resizes if buffer size is known before the call.
 * **callback?**: The function called when `.execute` has finished execution. If no callback function is given, `.execute` will return a native JavaScript `Promise`. Callback signature is:
     * error: The error that occured in execution, or `null` if no error
     * result: The [result array](#result-array) returned from the executed statement

src/odbc_statement.cpp

@@ -316,6 +316,43 @@ class ExecuteAsyncWorker : public ODBCAsyncWorker {
 
       SQLRETURN return_code;
 
+      // set SQL_ATTR_QUERY_TIMEOUT
+      if (data->query_options.timeout > 0) {
+        return_code =
+        SQLSetStmtAttr
+        (
+          data->hstmt,
+          SQL_ATTR_QUERY_TIMEOUT,
+          (SQLPOINTER) data->query_options.timeout,
+          IGNORED_PARAMETER
+        );
+
+        // It is possible that SQLSetStmtAttr returns a warning with SQLSTATE
+        // 01S02, indicating that the driver changed the value specified.
+        // Although we never use the timeout variable again (and so we don't
+        // REALLY need it to be correct in the code), its just good to have
+        // the correct value if we need it.
+        if (return_code == SQL_SUCCESS_WITH_INFO)
+        {
+          return_code =
+          SQLGetStmtAttr
+          (
+            data->hstmt,
+            SQL_ATTR_QUERY_TIMEOUT,
+            (SQLPOINTER) &data->query_options.timeout,
+            SQL_IS_UINTEGER,
+            IGNORED_PARAMETER
+          );
+        }
+
+        // Both of the SQL_ATTR_QUERY_TIMEOUT calls are combined here
+        if (!SQL_SUCCEEDED(return_code)) {
+          this->errors = GetODBCErrors(SQL_HANDLE_STMT, data->hstmt);
+          SetError("[odbc] Error setting the query timeout on the statement\0");
+          return;
+        }
+      }
+
       return_code =
       set_fetch_size
       (

test/statement/execute.test.js

@@ -251,6 +251,23 @@ describe('.execute([calback])...', () => {
         });
       });
     });
+    it.only('...should accept a timeout option without crashing', (done) => {
+      connection.createStatement((error1, statement) => {
+        assert.deepEqual(error1, null);
+        assert.notDeepEqual(statement, null);
+        statement.prepare(`SELECT * FROM ${process.env.DB_SCHEMA}.${process.env.DB_TABLE}`, (error2) => {
+          assert.deepEqual(error2, null);
+          statement.execute({timeout: 10}, (error3, result) => {
+            assert.deepEqual(error3, null);
+            assert.notDeepEqual(result, null);
+            statement.close((error4) => {
+              assert.deepEqual(error4, null);
+              done();
+            });
+          });
+        });
+      });
+    });
   }); // '...with callbacks...'
   describe('...with promises...', () => {
     it('...should execute if a valid SQL string has been prepared and valid values bound.', async () => {
@@ -366,5 +383,13 @@ describe('.execute([calback])...', () => {
       assert.deepEqual(statementResult[0].AGE, 10);
       await statement.close();
     });
+    it.only('...should accept a timeout option without crashing', async () => {
+      const statement = await connection.createStatement();
+      assert.notDeepEqual(statement, null);
+      await statement.prepare(`SELECT * FROM ${process.env.DB_SCHEMA}.${process.env.DB_TABLE}`);
+      const result = await statement.execute({timeout: 10});
+      assert.notDeepEqual(result, null);
+      await statement.close();
+    });
   }); // '...with promises...'
 });