Sql DataReader Has Rows but Returns With Empty data

Question

I am using a query to select data from sql database. It has rows but data raader.Read() function returns false and rows are empty as I have checked in debugging

Code that i have been using is

public void getSale()
    {
        DB db = new DB();

        try
        {
            db.cmd.CommandText = "select * from Sale where date is not null and (date between '"+StartDate+"' and '"+EndDate+"') order by date";
            db.cmd.Connection = db.con;
            db.con.Open();

            if(db.con.State == System.Data.ConnectionState.Open)
            {
                db.dataReader = db.cmd.ExecuteReader();

                if(db.dataReader.HasRows)
                {
                    while(db.dataReader.Read())
                    {
                        SaleModel sm = new SaleModel();
                        sm.SaleId = long.Parse(db.dataReader["Id"].ToString());
                        sm.UserName = db.dataReader["UserName"].ToString();
                        sm.ItemsQuantity = int.Parse(db.dataReader["ItemsQuantity"].ToString());
                        sm.TotalAmount = double.Parse(db.dataReader["TotalAmount"].ToString());
                        sm.SubTotal = double.Parse(db.dataReader["SubTotal"].ToString());
                        sm.Discount = double.Parse(db.dataReader["Discount"].ToString());
                        sm.Completed = bool.Parse(db.dataReader["Completed"].ToString());
                        sm.Date = DateTime.Parse(db.dataReader["Date"].ToString());
                        sm.CustomerPhone = long.Parse(db.dataReader["CustomerPhone"].ToString());

                        SalesList.Add(sm);
                    }

                    db.con.Close();
                }
            }
        }
        catch(Exception ex)
        {
            MessageBox.Show(ex.Message, "Exception", MessageBoxButton.OK, MessageBoxImage.Error, MessageBoxResult.OK);
        }
    }

And When I tested this query on Query editor in Visual studio rows were returned

If Anyone can help?

Yes, I know I have very little time and I can edit and add sqlparameters later so, that not the issue with code here. — kirito70, Jun 14 '17 at 14:08
What makes you think parameters will take longer to code? Anyway, you'll have less time if you have to go back and change something that already works, or clean up after a hack. I've also seen too many cases where a subtle formatting issue is the cause of issues like this, where using parameters in the first place would have avoided this mess. — Joel Coehoorn, Jun 14 '17 at 14:10
Use parameters for DateTimes. The database probably has is using DateTime for the columns and you are using strings in the query which won't work.. — jdweng, Jun 14 '17 at 14:10
I was having a parameter length issue on which one of the resultsets was coming blank on vb.net side from sql server. when I fixed it, it worked like a charm. — dinesh kumar, Jun 11 '21 at 04:41

Tim Schmelter · Accepted Answer · 2017-06-14T14:20:02.280

Why you concatenate strings to build your sql query? NEVER do that. It is a source for sql-injection and can cause issues like this. Instead use parameterized queries.

Also don't use SqlConnection wrappers like your DB class. That can cause several other issues. Instead create, open, close and dispose them where you need them, best by using the using-statament. The connection-pooling will manage the rest for you.

public List<SaleModel> GetSale(DateTime startDate, DateTime endDate)
{
    string sql = @"select * from Sale 
                   where date is not null 
                   and date between @StartDate and @EndDate 
                   order by date";

    var salesList = new List<SaleModel>();

    try
    {
        using (var con = new SqlConnection("insert your connection string"))
        using (var cmd = new SqlCommand(sql, con))
        {
            cmd.Parameters.Add("@StartDate", SqlDbType.DateTime).Value = startDate;
            cmd.Parameters.Add("@EndDate", SqlDbType.DateTime).Value = endDate;
            con.Open();
            using (var reader = cmd.ExecuteReader())
            {
                while (reader.Read())
                {
                    SaleModel sm = new SaleModel();
                    sm.SaleId = long.Parse(reader["Id"].ToString());
                    sm.UserName = reader["UserName"].ToString();
                    sm.ItemsQuantity = int.Parse(reader["ItemsQuantity"].ToString());
                    sm.TotalAmount = double.Parse(reader["TotalAmount"].ToString());
                    sm.SubTotal = double.Parse(reader["SubTotal"].ToString());
                    sm.Discount = double.Parse(reader["Discount"].ToString());
                    sm.Completed = bool.Parse(reader["Completed"].ToString());
                    sm.Date = DateTime.Parse(reader["Date"].ToString());
                    sm.CustomerPhone = long.Parse(reader["CustomerPhone"].ToString());

                    salesList.Add(sm);
                }
            }
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message, "Exception", MessageBoxButton.OK, MessageBoxImage.Error, MessageBoxResult.OK);
    }

    return salesList;
}

I'm pretty sure that this works(f.e. can be a localization issue).

Side-note: a method GetSale should return a List<SaleModel> but not fill one. You should also pass the parameters as DateTime to the method. I've changed it in my code sample.

That worked. Thanks I will be more care full and will use SqlParameters — kirito70, Jun 14 '17 at 14:41
I have a confusion that if I am going to add Connection String every time and POS that I am working there are a lot of connections so that won't be possible if I don't make an abstract class like DB. And More I am asking user at the start to specify storage for DB? So, How it is possible — kirito70, Jun 14 '17 at 14:44
@kirito70: configuration files are made for this purpose, f.e. the `App.Config`. Store the connection string there and read it where you innitialize the SqlConnection. Since config classes are cached there is no cost in reading it always. If you ask the user you want to make this config writable. That is possible(even on user base). But of course you can also have your own settings-class that stores such informations. But dont store ADO.NET objects there like the connection itself. — Tim Schmelter, Jun 14 '17 at 14:46
@TimSchmelter can I ask why in the answer the entire sql string has an "at" sign in front of it? I understand placing this in front of the separate parameters like startDate, but have not seen it placed before the entire string before the double quotes. Thanks! — David Mays, Jul 14 '21 at 20:33
@DavidMays: that's a [verbatim string literal](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/verbatim) which enables you to write a string literal on multiple lines. — Tim Schmelter, Jul 14 '21 at 20:36

Joel Coehoorn · Answer 2 · 2017-06-14T14:46:23.107

This is a much better way to structure you code, and there's a good chance it will fix your issue, too:

//accept the date values as parameter arguments, return the result. 
//  Do NOT mess about with variables at the global or class scope.
public IEnumerable<SalesModel> getSale(DateTime StartDate, DateTime EndDate)
{
    string sql = "select * from Sale where date is not null and (date between @StartDate and @EndDate) order by date";

    //DON'T abstract SqlCommand/SqlConnection. DO abstract your connection string.
    //Also, don't bother with the try/catch at this level. You can't really do anything with it here, so worry about the exception in calling code.
    using (var cn = new SqlConnection(DB.ConnectionString))
    using (var cmd = new SqlCommand(sql, cn))
    {
        cmd.Parameters.Add("@StartDate", SqlDbType.DateTime).Value = StartDate
        cmd.Parameters.Add("@EndDate", SqlDbType.DateTime).Value = EndDate
        cn.Open();

        using (SqlDataReader rdr = cmd.ExecuteReader())
        {
            while(rdr.Read())
            {
                var sm = new SaleModel();
                //If you have good schema design, these values are **already** in the correct type. 
                // The old code forces an expensive conversion to string, following by an expensive parse back to the type it already had.
                // We can do MUCH better.

                sm.SaleId = (long)rdr["Id"];
                //but it is okay for types that are *already* strings
                sm.UserName = rdr["UserName"].ToString();
                sm.ItemsQuantity = (int)rdr["ItemsQuantity"];
                sm.TotalAmount = (double)rdr["TotalAmount"]);
                sm.SubTotal = (double)rdr["SubTotal"];
                sm.Discount = (double)rdr["Discount"];
                sm.Completed = (bool)rdr["Completed"];
                sm.Date = (DateTime)rdr["Date"];
                sm.CustomerPhone = (long).rdr["CustomerPhone"];

                yield return sm;
            }
        }
    }
}

Here it is again without all the extra comments. The point here is this is still less code than the original that used string concatenation, and it took less than 10 minutes to write. Good code doesn't necessarily take longer.

public IEnumerable<SalesModel> getSale(DateTime StartDate, DateTime EndDate)
{
    string sql = "select * from Sale where date is not null and (date between @StartDate and @EndDate) order by date";

    using (var cn = new SqlConnection(DB.ConnectionString))
    using (var cmd = new SqlCommand(sql, cn))
    {
        cmd.Parameters.Add("@StartDate", SqlDbType.DateTime).Value = StartDate
        cmd.Parameters.Add("@EndDate", SqlDbType.DateTime).Value = EndDate
        cn.Open();

        using (SqlDataReader rdr = cmd.ExecuteReader())
        {
            while(rdr.Read())
            {
                var sm = new SaleModel();
                sm.SaleId = (long)rdr["Id"];
                sm.UserName = rdr["UserName"].ToString();
                sm.ItemsQuantity = (int)rdr["ItemsQuantity"];
                sm.TotalAmount = (double)rdr["TotalAmount"]);
                sm.SubTotal = (double)rdr["SubTotal"];
                sm.Discount = (double)rdr["Discount"];
                sm.Completed = (bool)rdr["Completed"];
                sm.Date = (DateTime)rdr["Date"];
                sm.CustomerPhone = (long).rdr["CustomerPhone"];

                yield return sm;
            }
        }
    }
}

Note that I return an IEnumerable rather than a List. If you really need a List (tip: you probably don't, and sticking with IEnumerable is faster), you can just call ToList() on the result.

Is there a way that sql database will return in Specified types already? — kirito70, Jun 14 '17 at 14:49
The `DbDataReader`'s index property is of type `Object`, but the values inside are **already** the types you need. The casts in this code don't do any work at runtime. They just tell the compiler that these assignments are "safe" and legal. Note that if you need to deal with NULLs from the database, things are a bit more complicated. — Joel Coehoorn, Jun 14 '17 at 14:55

score 0 · Answer 3 · answered Jun 14 '17 at 14:08

don't see any issue except that you are using date which is a reserve word and not your actual column name. Change your query to be

db.cmd.CommandText = "select * from Sale where [date] is not null and ([date] between '"+StartDate+"' and '"+EndDate+"') order by [date]";

Sql DataReader Has Rows but Returns With Empty data

3 Answers3