Found a bug with batch statements that don't contain a select.

Jeff Farr <[email protected]> Thu, 17 Nov 2016 17:35:05 +0000
Newsgroups gmane.comp.db.tds.freetds
Message-ID <[email protected]>
Hello all, a while back we found some unexpected behavior when sending multiple queries in a statement and, initially, thought it was a bug with PHPs PDO library (you can see the bug report here https://bugs.php.net/bug.php?id=72969).  I dug into the issue and found that the underlying problem is that if you call dbresults on a result set that doesn't contain TDS_ROW_RESULT or TDS_ROWFMT_RESULT then the for loop will process every set of tokens in the data in one swoop and then return NO_MORE_ROWS to the calling function.


I've made a small change to _dbresults and added a couple of unit tests that demonstrate the issue and verify that it's fixed, review and feedback from the list is much appreciated!


If it's easier, I also created a PR against Branch-1_00 here: https://github.com/FreeTDS/freetds/pull/84


Thanks,

-Jeff

_______________________________________________
FreeTDS mailing list
[email protected]
http://lists.ibiblio.org/mailman/listinfo/freetds
batch_statements_without_select.diff (application/octet-stream, 15 KB)
diff --git a/src/dblib/dblib.c b/src/dblib/dblib.c
index 0cddda7c9ce7af01864d65e333ea14aa739eca02..9916204140cd119941dd1be229638998b235daaa 100644
--- a/src/dblib/dblib.c
+++ b/src/dblib/dblib.c
@@ -1766,6 +1766,8 @@ _dbresults(DBPROCESS * dbproc)
 					dbproc->dbresults_state = _DB_RES_NEXT_RESULT;
 					if (done_flags & TDS_DONE_ERROR)
 						return FAIL;
+					if (result_type == TDS_DONE_RESULT)
+						return SUCCEED;
 					break;
 
 				case _DB_RES_RESULTSET_EMPTY:
diff --git a/src/dblib/unittests/wf_dbresults.c b/src/dblib/unittests/wf_dbresults.c
new file mode 100644
index 0000000000000000000000000000000000000000..db4978c50e894da0ad50d49fcd486e772b417470
--- /dev/null
+++ b/src/dblib/unittests/wf_dbresults.c
@@ -0,0 +1,197 @@
+/*
+ * Purpose:
+ * Functions: dbbind dbcmd dbcolname dberrhandle dbisopt dbmsghandle dbnextrow dbnumcols dbopen dbresults dbsetlogintime dbsqlexec dbuse
+ */
+
+#include "common.h"
+
+static char software_version[] = "$Id: wf_dbresults.c,v 1.29 2016-11-03 15:52:48 freddy77 Exp $";
+static void *no_unused_var_warn[] = { software_version, no_unused_var_warn };
+
+
+
+int failed = 0;
+
+
+int
+main(int argc, char **argv)
+{
+  const int rows_to_add = 50;
+  LOGINREC *login;
+  DBPROCESS *dbproc;
+  int i;
+  char teststr[1024];
+  DBINT testint, erc;
+
+  set_malloc_options();
+
+  read_login_info(argc, argv);
+  if (argc > 1) {
+    argc -= optind;
+    argv += optind;
+  }
+
+  fprintf(stdout, "Starting %s\n", argv[0]);
+
+  /* Fortify_EnterScope(); */
+  dbinit();
+
+  dberrhandle(syb_err_handler);
+  dbmsghandle(syb_msg_handler);
+
+  fprintf(stdout, "About to logon as \"%s\"\n", USER);
+
+  login = dblogin();
+  DBSETLPWD(login, PASSWORD);
+  DBSETLUSER(login, USER);
+  DBSETLAPP(login, "wf_dbresults");
+
+  if (argc > 1) {
+    printf("server and login timeout overrides (%s and %s) detected\n", argv[0], argv[1]);
+    strcpy(SERVER, argv[0]);
+    i = atoi(argv[1]);
+    if (i) {
+      i = dbsetlogintime(i);
+      printf("dbsetlogintime returned %s.\n", (i == SUCCEED)? "SUCCEED" : "FAIL");
+    }
+  }
+
+  fprintf(stdout, "About to open \"%s\"\n", SERVER);
+
+  dbproc = dbopen(login, SERVER);
+  if (!dbproc) {
+    fprintf(stderr, "Unable to connect to %s\n", SERVER);
+    return 1;
+  }
+  dbloginfree(login);
+
+  fprintf(stdout, "Using database \"%s\"\n", DATABASE);
+  if (strlen(DATABASE)) {
+    erc = dbuse(dbproc, DATABASE);
+    assert(erc == SUCCEED);
+  }
+
+  RETCODE ret;
+  int rowcount;
+  int colcount;
+
+  // First, call everything that happens in PDO::query
+  // dblib_stmt.c pdo_dblib_stmt_execute
+  //    pdo_dblib_stmt_cursor_closer will call this
+  dbcancel(dbproc);
+
+  //fprintf(stdout, "using dbcmd\n");
+  //dbcmd(dbproc, "create table #wf_dbresults(id int);insert into #wf_dbresults values(1), (2), (3);update #wf_dbresults set id = 1;insert into #wf_dbresults values(2);drop table #wf_dbresults;");
+
+  fprintf(stdout, "using sql_cmd\n");
+  sql_cmd(dbproc);
+  dbsqlexec(dbproc);
+
+  //    pdo_dblib_stmt_next_rowset_no_cancel
+  ret = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", ret);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  // end dblib_stmt.c pdo_dblib_stmt_execute
+  assert(ret == SUCCEED);
+  assert(rowcount == -1);
+  assert(colcount == 0);
+
+  // now simulate calling nextRowset()
+  // this should return the results of the first INSERT
+
+  // we don't expect any rows back from these statements
+  ret = dbnextrow(dbproc);
+  assert(ret == NO_MORE_ROWS);
+
+  ret = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", ret);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(ret == SUCCEED);
+  assert(rowcount == 3);
+  assert(colcount == 0);
+
+  // this should return the results of the UPDATE
+  while (NO_MORE_ROWS != ret) {
+    ret = dbnextrow(dbproc);
+
+    if (FAIL == ret) {
+      fprintf(stderr, "dbnextrows returned FAIL\n");
+      return 1;
+    }
+  }
+
+  ret = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", ret);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(ret == SUCCEED);
+  assert(rowcount == 3);
+  assert(colcount == 0);
+
+  // this should return the results of the second INSERT
+  ret = dbnextrow(dbproc);
+  assert(ret == NO_MORE_ROWS);
+
+  ret = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", ret);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(ret == SUCCEED);
+  assert(rowcount == 1);
+  assert(colcount == 0);
+
+  // this should return the results of the DROP
+  ret = dbnextrow(dbproc);
+  assert(ret == NO_MORE_ROWS);
+
+  ret = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", ret);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(ret == SUCCEED);
+  assert(rowcount == -1);
+  assert(colcount == 0);
+
+  // Call one more time to be sure we get NO_MORE_RESULTS
+  ret = dbnextrow(dbproc);
+  assert(ret == NO_MORE_ROWS);
+
+  ret = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", ret);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(ret == NO_MORE_RESULTS);
+  assert(rowcount == -1);
+  assert(colcount == 0);
+
+  dbexit();
+
+  fprintf(stdout, "%s %s\n", __FILE__, (failed ? "failed!" : "OK"));
+  return failed ? 1 : 0;
+}
diff --git a/src/dblib/unittests/wf_dbresults.sql b/src/dblib/unittests/wf_dbresults.sql
new file mode 100644
index 0000000000000000000000000000000000000000..d6ed377184bb7a98a0a16af77a5d70d6e9953c7f
--- /dev/null
+++ b/src/dblib/unittests/wf_dbresults.sql
@@ -0,0 +1,5 @@
+create table #wf_dbresults(id int);
+insert into #wf_dbresults values(1), (2), (3);
+update #wf_dbresults set id = 1;
+insert into #wf_dbresults values(2);
+drop table #wf_dbresults;
diff --git a/src/dblib/unittests/wf_insert_select.c b/src/dblib/unittests/wf_insert_select.c
new file mode 100644
index 0000000000000000000000000000000000000000..56bd1aa3c06d3c8ec4b28fc085bbf297dd9e408a
--- /dev/null
+++ b/src/dblib/unittests/wf_insert_select.c
@@ -0,0 +1,258 @@
+/*
+ * Purpose:
+ * Functions: dbbind dbcmd dbcolname dberrhandle dbisopt dbmsghandle dbnextrow dbnumcols dbopen dbresults dbsetlogintime dbsqlexec dbuse
+ */
+
+#include "common.h"
+
+static char software_version[] = "$Id: wf_insert_select.c,v 1.29 2016-11-03 15:52:48 freddy77 Exp $";
+static void *no_unused_var_warn[] = { software_version, no_unused_var_warn };
+
+
+
+int failed = 0;
+
+
+int
+main(int argc, char **argv)
+{
+  LOGINREC *login;
+  DBPROCESS *dbproc;
+  int i;
+  DBINT erc;
+
+  set_malloc_options();
+
+  read_login_info(argc, argv);
+  if (argc > 1) {
+    argc -= optind;
+    argv += optind;
+  }
+
+  fprintf(stdout, "Starting %s\n", argv[0]);
+
+  /* Fortify_EnterScope(); */
+  dbinit();
+
+  dberrhandle(syb_err_handler);
+  dbmsghandle(syb_msg_handler);
+
+  fprintf(stdout, "About to logon as \"%s\"\n", USER);
+
+  login = dblogin();
+  DBSETLPWD(login, PASSWORD);
+  DBSETLUSER(login, USER);
+  DBSETLAPP(login, "wf_dbresults");
+
+  if (argc > 1) {
+    printf("server and login timeout overrides (%s and %s) detected\n", argv[0], argv[1]);
+    strcpy(SERVER, argv[0]);
+    i = atoi(argv[1]);
+    if (i) {
+      i = dbsetlogintime(i);
+      printf("dbsetlogintime returned %s.\n", (i == SUCCEED)? "SUCCEED" : "FAIL");
+    }
+  }
+
+  fprintf(stdout, "About to open \"%s\"\n", SERVER);
+
+  dbproc = dbopen(login, SERVER);
+  if (!dbproc) {
+    fprintf(stderr, "Unable to connect to %s\n", SERVER);
+    return 1;
+  }
+  dbloginfree(login);
+
+  fprintf(stdout, "Using database \"%s\"\n", DATABASE);
+  if (strlen(DATABASE)) {
+    erc = dbuse(dbproc, DATABASE);
+    assert(erc == SUCCEED);
+  }
+
+  RETCODE results_retcode;
+  int rowcount;
+  int colcount;
+  int row_retcode;
+
+  /*
+  * This test is written to simulate how dblib is used in PDO
+  * functions are called in the same order they would be if doing
+  * PDO::query followed by some number of PDO::statement->nextRowset
+  */
+
+  // First, call everything that happens in PDO::query
+  // this will return the results of the CREATE TABLE statement
+  dbcancel(dbproc);
+
+  fprintf(stdout, "using sql_cmd\n");
+  sql_cmd(dbproc);
+  dbsqlexec(dbproc);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "** CREATE TABLE **\n");
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  // check that the results correspond to the create table statement
+  assert(results_retcode == SUCCEED);
+  assert(rowcount == -1);
+  assert(colcount == 0);
+
+  // now simulate calling nextRowset() for each remaining statement in our batch
+
+  // --------------------------------------------------------------------------
+  // INSERT
+  // --------------------------------------------------------------------------
+  fprintf(stdout, "** INSERT **\n");
+
+  // there shouldn't be any rows in this resultset yet, it's still from the CREATE TABLE
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", results_retcode);
+  assert(row_retcode == NO_MORE_ROWS);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(results_retcode == SUCCEED);
+  assert(rowcount == 3);
+  assert(colcount == 0);
+
+  // --------------------------------------------------------------------------
+  // SELECT
+  // --------------------------------------------------------------------------
+  fprintf(stdout, "** SELECT **\n");
+
+  // the rowset is still from the INSERT and should have no rows
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", results_retcode);
+  assert(row_retcode == NO_MORE_ROWS);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(results_retcode == SUCCEED);
+  assert(rowcount == -1);
+  assert(colcount == 1);
+
+  // now we expect to find three rows in the rowset
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == REG_ROW); // 4040 corresponds to TDS_ROW_RESULT in freetds/tds.h
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == REG_ROW);
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n\n", row_retcode);
+  assert(row_retcode == REG_ROW);
+
+  // --------------------------------------------------------------------------
+  // UPDATE
+  // --------------------------------------------------------------------------
+  fprintf(stdout, "** UPDATE **\n");
+
+  // check that there are no rows left, then we'll get the results from the UPDATE
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == NO_MORE_ROWS);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(results_retcode == SUCCEED);
+  assert(rowcount == 3);
+  //assert(colcount == 0); // TODO: why does an update get a column?
+
+  // --------------------------------------------------------------------------
+  // SELECT
+  // --------------------------------------------------------------------------
+  fprintf(stdout, "** SELECT **\n");
+
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == NO_MORE_ROWS);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(results_retcode == SUCCEED);
+  assert(rowcount == -1);
+  assert(colcount == 1);
+
+  // now we expect to find three rows in the rowset again
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == REG_ROW);
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == REG_ROW);
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n\n", row_retcode);
+  assert(row_retcode == REG_ROW);
+
+  // --------------------------------------------------------------------------
+  // DROP
+  // --------------------------------------------------------------------------
+  fprintf(stdout, "** DROP **\n");
+
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == NO_MORE_ROWS);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(results_retcode == SUCCEED);
+  assert(rowcount == -1);
+  //assert(colcount == 1);
+
+  // Call one more time to be sure we get NO_MORE_RESULTS
+  row_retcode = dbnextrow(dbproc);
+  fprintf(stdout, "dbnextrow retcode: %d\n", row_retcode);
+  assert(row_retcode == NO_MORE_ROWS);
+
+  results_retcode = dbresults(dbproc);
+  rowcount = DBCOUNT(dbproc);
+  colcount = dbnumcols(dbproc);
+
+  fprintf(stdout, "RETCODE: %d\n", results_retcode);
+  fprintf(stdout, "ROWCOUNT: %d\n", rowcount);
+  fprintf(stdout, "COLCOUNT: %d\n\n", colcount);
+
+  assert(results_retcode == NO_MORE_RESULTS);
+  assert(rowcount == -1);
+  //assert(colcount == 0);
+
+  dbexit();
+
+  fprintf(stdout, "%s %s\n", __FILE__, (failed ? "failed!" : "OK"));
+  return failed ? 1 : 0;
+}
diff --git a/src/dblib/unittests/wf_insert_select.sql b/src/dblib/unittests/wf_insert_select.sql
new file mode 100644
index 0000000000000000000000000000000000000000..42316bd97c943da140a011d8f9c8c47edce86efc
--- /dev/null
+++ b/src/dblib/unittests/wf_insert_select.sql
@@ -0,0 +1,6 @@
+create table #wf_insert_select(id int);
+insert into #wf_insert_select values(1), (2), (3);
+select * from #wf_insert_select;
+update #wf_insert_select set id = 4;
+select * from #wf_insert_select;
+drop table #wf_insert_select;