What changes may be made to my GetEmails() method to decrease database execution time?

Question

The code is like this:

public class Email
{
    public string MailAddress {get;set;}
    public string MailOwner {get;set;}
    public int MailSended {get;set;}
    public int MailReceived {get;set;}
}

public class EmailList
{
    public List<Email> Items {get; private set;}
    private IDBConnection _connection;


    public EmailList(IDBConnection connection)
    {
        _connection = connection;
    }

    public List<Email> GetEmails(int customerId)
    {
        var emailList = new List<Email>();
        var sql = "SELECT * FROM EmailsAddress WHERE Id = '" + customerId + "'";

        using (var ObjectToAccessDatabaseAndStoreResultsIntoDataTable = new ObjectToObtainDbResultsIntoDataTable(_connection, sql))
        {
            foreach(DataRow row in  ObjectToAccessDatabaseAndStoreResultsIntoDataTable.Datatable)
            {
                var email = new Email{
                MailAddress = Convert.ToString(row["MailAddress"]);
                MailOwner = Convert.ToString(row["MailOwner"]);
                MailSended = Convert.ToInt32(row["MailSended"]);
                MailReceived = Convert.ToInt32(row["MailReceived"]);
            }

            emailList.Add(email);
        }       
        return emailList;
    }
}

When GetEmails() is called multiple times to retrieve mail addresses, for i.e. 2000 customers, it takes a very long time to return. Using SQL Server Profiler, I see that most of the time is for the GetEmails() routine.

Why is the GetEmails() routine taking so long? I am using SQL Server.

It doesn't seem right to get email address by id, shouldn't that be something like `where CustomerId = ...`? — Silvermind, Apr 05 '17 at 08:30
I do a quick example code. Obiviuosly my code is not like this but it work like this. — CRK, Apr 05 '17 at 08:32

Christoph Sonntag · Accepted Answer · 2017-04-05T18:16:21.583

2

Does the Id-column in your EmailsAddress-table have an index on this column? If not, adding one will speed it up.
You always should use parametrized queries and not string concatination - it allows the database to optimize your queries and, even more important, protects you from sql injection attacks (although not that relevant in your case with an integer type).
```
 var command =
    new SqlCommand("SELECT * FROM EmailsAddress WHERE Id = @Id;", db);
command.Parameters.AddWithValue("@Id", yourTextValue);
command.ExecuteQuery();
```
What Patrick Hofman said, getting your customers mailaddresses by grouping the IDs or using a join when querying the customers will be way more efficient.

/Edit: Here an example for a query asking for multiple ids :

var command =
    new SqlCommand("SELECT * FROM EmailsAddress WHERE Id IN (@Id1,@Id2,@Id3);", db);
command.Parameters.AddWithValue("@Id1", 1);
command.Parameters.AddWithValue("@Id2", 54);
command.Parameters.AddWithValue("@Id2", 96);
command.ExecuteReader();

Or, ignoring my own advice to use parameters :

var sql = "SELECT * FROM EmailsAddress WHERE Id IN (1,54,96)";

Example for doing this in a loop, with parameters :

        var ids = new int[] { 1, 95, 46, 93, 98, 77 };
        var isFirst = true;
        var commandBuilder = new StringBuilder("SELECT * FROM EmailsAddress WHERE Id IN (");
        var command = new SqlCommand("");
        for (var i = 0;i<ids.Length;i++)
        {
            if (!isFirst) commandBuilder.Append(",");
            else isFirst = false;
            var paramName = "@Id" + i;
            commandBuilder.Append(paramName);
            command.Parameters.AddWithValue(paramName, ids[i]);
        }
        commandBuilder.Append(")");
        command.CommandText = commandBuilder.ToString();
        command.ExecuteReader();

        // Read your result

edited Apr 05 '17 at 18:16

answered Apr 05 '17 at 08:33

Christoph Sonntag

4,459
1
24
49

can you make an example? what do you mean by "grouping id"? if I'm able to group id, how to pass this group to the query? For example I want emails from 2000 customer that not have sequencial IDs (1, 54, 1450 and so on) so it can't be a range. – CRK Apr 05 '17 at 08:37
I added an example. – Christoph Sonntag Apr 05 '17 at 08:41
What if I have 200 IDs? – CRK Apr 05 '17 at 08:42
Yes, this would be the easiest option (of cause you would do this in a loop and not manually). For more information on the limits for this kind of query and some alternatives, look here : http://stackoverflow.com/questions/1869753/maximum-size-for-a-sql-server-query-in-clause-is-there-a-better-approach - especially interesting is the 2.100 parameters per query limit, this would need splitting your query into multiple queries. But if you get such a huge amount of parameters, you should consider using a join instead. – Christoph Sonntag Apr 05 '17 at 08:44
and what if I want parameters and add them with loop? please do an example. I'm not expert of SQL. – CRK Apr 05 '17 at 08:46
This is not an issue of SQL, this is pure C# :> But give me a minute, I'll provide an example. – Christoph Sonntag Apr 05 '17 at 08:48
I know but how to add mutiple parameters automatically into a sql command? Ok, thank you – CRK Apr 05 '17 at 08:49

score 1 · Answer 2 · answered Apr 05 '17 at 08:29

1

Can you find where's the problem?

Yes, you are firing 2000 SQL statements to the database, which has to parse and execute them all one-by-one, while one SQL statement could suffice in this case.

What you could do, depending on the size of the database:

Load all records at once. Store them in a cache object, and answer from that cache. This can be a problem when there are a lot of rows and you only intend to get a small set of records;
Load multiple customers at once. This might help if you don't want to get the entire table at once, but do have a tight loop which repeatedly calls the GetEmails method. You could pass in a list of customer IDs and fetch those at once. This would require an in statement on your customerId.

answered Apr 05 '17 at 08:29

Patrick Hofman

153,850
22
249
325

So the problem is not the multiple instance of the class Email? Can you do an example with code? – CRK Apr 05 '17 at 08:34
No. Those instances are not the problem. I can't give examples for all of the points mentioned. The documentation for each product will help you for sure. – Patrick Hofman Apr 05 '17 at 08:36
Can you do an example at least for the second point? I can't understand what you mean – CRK Apr 05 '17 at 08:40
Instead of `SELECT * FROM EmailsAddress WHERE Id = 1` you could do `SELECT * FROM EmailsAddress WHERE Id in (1, 2, 3, 4, 40, 100)` – Patrick Hofman Apr 05 '17 at 08:41

score 1 · Answer 3 · answered Apr 05 '17 at 08:42

You can get email address for list customers like this

 public List<Email> GetEmails(List<int> listCustomerId)
    {
        var emailList = new List<Email>();
        var sql = "SELECT * FROM EmailsAddress WHERE Id IN (" + string.Join(",", listCustomerId) + ")";

        using ( var ObjectToAccessDatabaseAndStoreResultsIntoDataTable = new ObjectToObtainDbResultsIntoDataTable(_connection, sql))
        {
            foreach (DataRow row in  ObjectToAccessDatabaseAndStoreResultsIntoDataTable.Datatable)
            {
                var email = new Email
                {
                    MailAddress = Convert.ToString(row["MailAddress"]);
                    MailOwner = Convert.ToString(row["MailOwner"]);
                    MailSended = Convert.ToInt32(row["MailSended"]);
                    MailReceived = Convert.ToInt32(row["MailReceived"]);
                }

                emailList.Add(email);
            }

            return emailList;
        }
    }

What changes may be made to my GetEmails() method to decrease database execution time?

3 Answers3