Get a single record from SQL Server the correct way

Question

I'm using Ado to retrieve a single record by id. Observe:

public async Task<Image> GetImage(int id)
{
    var image = new Image();

    using (SqlConnection conn = new SqlConnection(ConnectionString))
    {
        conn.Open();

        string sql = @" SELECT * FROM Images where id = @id";

        using (SqlCommand comm = new SqlCommand(sql, conn))
        {
            comm.Parameters.AddWithValue("@id", id);

            var reader = await comm.ExecuteReaderAsync();

            int ordId = reader.GetOrdinal("id");
            int ordName = reader.GetOrdinal("name");
            int ordPath = reader.GetOrdinal("path");

            while (reader.Read())
            {
                image.Id = reader.GetInt32(ordId);
                image.Name = reader.GetString(ordName);
                image.Path = reader.GetString(ordPath);
            }

            return image;
        }
    }
}

As you can see I am using While to iterate through the records. Since while is signifying there may be more than one record to iterate I believe this may be the wrong way to get get a single record. Considering ADO has ExecuteScalar for one row one field maybe they have a specified way for one row multiple fields. Is there a specified way to get a single record in ADO?

Of course it'll work as-is but to make it clear to who will read your source code you may replace `while(reader.Read())` with `if (reader.Read())` and (eventually add `TOP 1` to your `SELECT`, in case SQL Server will enable any query optimization because of that). Take also a look to LINQ `.Single()` implementation (for SQL or EF) for some inspiration. — Adriano Repetti, Apr 25 '15 at 12:16
The ExecuteScalar method is just a convenience method that returns the first column of the first row of the first resultset. So if you can do the same thing for multiple columns by executing Read once, check for a true result, and closing the reader immediately. — Dan Guzman, Apr 25 '15 at 12:55

Douglas · Accepted Answer · 2015-04-25T13:11:25.720

I would go with your current approach, except that I'd eliminate the while loop. If you want to ensure that only one record is returned, perform an extra Read to ensure it returns false. This is similar to the semantics of the LINQ Single operator.

if (!reader.Read())        
    throw new InvalidOperationException("No records were returned.");

image.Id = reader.GetInt32(ordId);
image.Name = reader.GetString(ordName);
image.Path = reader.GetString(ordPath);

if (reader.Read())
    throw new InvalidOperationException("Multiple records were returned.");

Assuming that the id column in your database is a primary key (unique), there is no need to specify a TOP clause in the SQL query; the SQL Server query optimizer would deduce that only at most one record is returned due to the WHERE clause. However, if you don't have a primary key or unique index/constraint on the id column, then you should issue a TOP (2) clause to restrict the number of returned rows. You should avoid using TOP (1) because you would be unable to detect (and raise an error for) extra matches.

string sql = @"SELECT TOP (2) * FROM Images WHERE id = @id"

`TOP` without an `ORDER BY` is moot - what "top 2" are you getting? Unless you **explicitly define** an `ORDER BY`, the ordering (and therefore the "first two rows") is arbitrary.... — marc_s, Apr 25 '15 at 13:32
@marc_s: You don't care which two you're getting, as long as you're able to confirm that you are, indeed, getting two. (You will be throwing an exception and discarding their contents anyway.) Entity Framework uses the same notion when converting `Single()` to SQL. — Douglas, Apr 25 '15 at 13:36

Eugene Podskal · Answer 2 · 2015-04-25T12:32:56.523

What if you just read once:

using (SqlConnection conn = new SqlConnection(ConnectionString))
{
    conn.Open();

    string sql = @" SELECT id, name, path FROM Images where id = @id";

    using (SqlCommand comm = new SqlCommand(sql, conn))
    {
        comm.Parameters.AddWithValue("@id", id);           

        using (var reader = await comm.ExecuteReaderAsync())
        {
            if (!reader.Read())
                 throw new Exception("Something is very wrong");

            int ordId = reader.GetOrdinal("id");
            int ordName = reader.GetOrdinal("name");
            int ordPath = reader.GetOrdinal("path");

            image.Id = reader.GetInt32(ordId);
            image.Name = reader.GetString(ordName);
            image.Path = reader.GetString(ordPath);

            return image;
        }
    }
}

P.S.: I have also changed select statement to select only required fields and wrapped reader in using statement.

Ehsan Sajjad · Answer 3 · 2015-04-25T15:00:43.387

3

You can use Top(1) in this case in your query to get only single record from database:

SELECT Top(1) * FROM Images 
where id = @id
order by id desc -- will get the latest record

edited Apr 25 '15 at 15:00

answered Apr 25 '15 at 12:16

Ehsan Sajjad

61,834
16
105
160

3

`TOP` without an `ORDER BY` is moot - what "top 1" are you getting? Unless you **explicitly define** an `ORDER BY`, the ordering (and therefore the "first row") is arbitrary.... – marc_s Apr 25 '15 at 13:32
2

@marc_s You persuaded the poor guy to add a redundant order by clause to his query! – cja Aug 03 '17 at 17:26

Get a single record from SQL Server the correct way

3 Answers3

Linked