3

The function below was designed to determine if a query will return any rows. The passed in SQL is the query. If an error results the function should return false. However when SQL =

SELECT TOP 1 [AU_ID] 
       FROM [dat].[model_80av2_v1_2941] 
       WHERE [AU_ID] IS NOT NULL AND convert(int, [AU_ID]) <> [AU_ID]

the function mistakenly returns true since no error was detected. However executing the same query in SQL Management Studio results in an error:

Msg 232, Level 16, State 3, Line 3 Arithmetic overflow error for type int, value = -1000000000000000000000000000000.000000.

Clearly the function should have returned false because a value falls outside of the int data range but the error handling detects no error. Why? From other posts my understanding is that SqlDataReader reader = cmd.ExecuteReader() should result in an error.

private bool GetIfExists(string SQL, out int ErrorNumber, out bool Exists)
{
    bool IsSuccess = true;
    ErrorNumber = 0;
    Exists = false;

    try
    {
        using (SqlConnection cnn = new SqlConnection(_connectionString))
        {
            try
            {
                cnn.Open();

                using (SqlCommand cmd = cnn.CreateCommand())
                {
                    cmd.CommandText = SQL;
                    cmd.CommandTimeout = _commandTimeout;
                    try
                    {
                        using (SqlDataReader reader = cmd.ExecuteReader())
                        {
                            Exists = reader.HasRows;
                        }
                    }
                    catch (SqlException ex)
                    {
                        if (ex.Errors.Count > 0) ErrorNumber = ex.Errors[0].Number;
                        throw;
                    }
                    catch
                    {
                        throw;
                    }
                }
            }
            catch
            {
                throw;
            }
            finally
            {
                cnn.Close();
            }
        }
    }
    catch
    {
        IsSuccess = false;
    }
    return IsSuccess;
}
Balagurunathan Marimuthu
  • 2,927
  • 4
  • 31
  • 44
Peter
  • 393
  • 3
  • 15
  • Why so many catch throw? Also, the point of the using statement is that you don't have to call stuff like SqlConnection.Close() yourself, which you are explicitly doing – Camilo Terevinto Jul 10 '17 at 11:31
  • @mjwillis The sql statament is in the question – Peter Jul 10 '17 at 11:33
  • @CamiloTerevinto The pure terror of connection pooling problems. Its probably overdone. – Peter Jul 10 '17 at 11:34
  • On a side note, you don't need `cnn.Close();` - the `using` will take care of that for you. – mjwills Jul 10 '17 at 11:37
  • 1
    @Peter, I think adding `reader.NextResult();` after `Exists = reader.HasRows;` will raise the error (http://www.dbdelta.com/the-curious-case-of-undetected-sql-exceptions/). Let me know and I'll elaborate with an answer. – Dan Guzman Jul 10 '17 at 11:43
  • @Dan Guzman Exactly correct! – Peter Jul 10 '17 at 11:53

2 Answers2

8

From other posts my understanding is that SqlDataReader reader = cmd.ExecuteReader() should result in an error.

That is generally true except when an error occurs during a SQL Batch that returns rows to the client. The exception might not get raised until all results are consumed. This is because SQL Server streams results back to the client over the tabular data stream protocol (TDS) and result set rows precede the exception returned by SQL Server. The error is not seen and raised by the client API until all the preceding results are consumed.

Below is one way you could consume and discard remaining results to ensure the exception is raised.

using (SqlDataReader reader = cmd.ExecuteReader())
{
    Exists = reader.HasRows;
    do
    {
        while (reader.Read()) { };
    } while (reader.NextResult());
}

This article includes additional cases where SQL exceptions might not get raised as expected. I'll add that you will see errors raised by SQL Server Management Studio with the same query because it sets the FireInfoMessageEventOnUserErrors SqlConnection property to true instead of relying on a SqlException to be raised, something not routinely done in application code (nor should it be).

Dan Guzman
  • 43,250
  • 3
  • 46
  • 71
0

SQL Management Studio is likely defaulting ARITH ABORT to on. C# does not.

SQL Management Studio settings

Have a squiz at https://dba.stackexchange.com/questions/2500/make-sqlclient-default-to-arithabort-on and How to SET ARITHABORT ON for connections in Linq To SQL .

Additionally, your below query is problematic since TOP 1 will give different results depending on what ordering is used. To give repeatable results you really need an ORDER BY clause. Without it, the query optimiser can, for example, decide to order the data by Column A for some queries (e.g. from C#) and by Column B for other queries (e.g. from SQL Management Studio).

SELECT TOP 1 [AU_ID] 
       FROM [dat].[model_80av2_v1_2941] 
       WHERE [AU_ID] IS NOT NULL AND convert(int, [AU_ID]) <> [AU_ID]
mjwills
  • 23,389
  • 6
  • 40
  • 63
  • @mjwillis While that could well be it, the field or column actually does contain non-integer values as well as the crazy -1E30 value. Yet there is no error and read.HasRows of course must return false? Well an error did result, it simply is not passed on and the error trap fails to detect that anything is amiss. – Peter Jul 10 '17 at 11:38
  • What is the value of `IsSuccess` (on the return line) in that case? true or false? And what is the value of `Exists`? – mjwills Jul 10 '17 at 11:39
  • @mjwillis IsSuccess = true. That is the main problem. No error was detected and it is then implied that all rows in the field queried are integers, but they are not. – Peter Jul 10 '17 at 11:41
  • @mjwillis That is the insult added to the injury, Exists = false. – Peter Jul 10 '17 at 11:55
  • And it definitely isn't going into the `catch (SqlException ex)` block? How did you go adding the `ORDER BY`? – mjwills Jul 10 '17 at 11:58