In order to debug some site issues, I installed XDebug on my webserver for PHP. After enabling it, the SqlSrv driver spit out dozens of hidden exceptions, effectively burying my real issue like a needle in a haystack. I spent days sorting through all the various exceptions and found out that all of them were a result of sections of code where the exception mechanism (try/catch) was used to define normal code behavior. I have strong feelings about exceptions, one of the main ones being that exceptions should only be used in cases where something bad happened and avoided where it is considered expected behavior in many cases.

I have created a patch that will replace the offending try/catch areas with alternate code that does not need such a mechanism, thus creating a nice debug landscape where real errors jump out at you rather than get hidden under an avalanche of "expected behavior" exceptions.

The changes to tableExists and fieldExists are fairly simple:

  public function tableExists($table) {
    $table_info = $this->getPrefixInfo($table);
    $sql_query = 'SELECT 1 AS table_exists FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = :schema_name AND TABLE_NAME = :table_name';
    $sql_args = array(
        ':schema_name' => $table_info['schema'],
        ':table_name' => $table_info['table'],
    );
    return $this->connection->query($sql_query, $sql_args)->fetch();
  }

  public function fieldExists($table, $field) {
    $sql_query = "SELECT COL_LENGTH('{$table}','$field') AS field_len FROM INFORMATION_SCHEMA.TABLES";
    $theRow = $this->connection->query($sql_query)->fetch();
    return !is_null($theRow->field_len);
  }

When trying to find an alternative for this next section inside $queryStatement->execute(), though, I found that it was difficult to come up with something that would allow me to know when to call fetch() or not. I decided to just check for the SELECT keyword at the start of the statement, but if anyone knows a better way, please share. =) This particular bit of code is what caused the majority of "normal exceptions" because quite a number of non-SELECT queries are run against at least the cache tables each page.
Try/catch Code

    try {
      $this->data = $statement->fetchAll(PDO::FETCH_ASSOC);
    }
    catch (Exception $e) {
      $this->data = array();
    }

Patch Code

    $this->data = array();
    // Only call fetch() if we are executing a SELECT statement.
    if (strncasecmp(ltrim($this->queryString, "\t\n\r\0\x0B ("),'SELECT ',7)==0) { // strlen('SELECT ')=7
      $this->data = $statement->fetchAll(PDO::FETCH_ASSOC);
    }

If the maintainers agree and put this patch code into the driver, great -- otherwise it can just stay here so others that try to debug PHP can use it to help find their issues faster.

CommentFileSizeAuthor
_sqlsrv-avoid_try_catch.patch1.97 KBUncle_Code_Monkey

Comments

  • 8094f1d committed on 8.x-1.x
    Issue #1977810: Prevent Try/Catch behaviour in fieldExists()
    
david_garcia’s picture

Status: Needs review » Closed (works as designed)

I took into account this in the 8.0.x version of the driver, at least for tableExists and fieldExists. I am not confident in the changes in Statement->execute().

Because the 7.0.x implementation has proved to work well, I don't think it is worth backporting.

If anyone want's to see this in the 7.0.x version of the driver, provide a patch and tests.