C# checking if record exists in SQL error

Question

I am using this code to check whether a value (guid1) already exists in the 'guid' table:

string selectString = "SELECT guid" + "FROM trafficScotland" + "WHERE guid = " + guid1;

SqlCommand myCommand = new SqlCommand(selectString, myConnection);
String strResult = String.Empty;
strResult = (String)myCommand.ExecuteScalar();

 if (strResult.Length == 0)

But at the

 strResult = (String)myCommand.ExecuteScalar();

line, I get the sqlException error

Incorrent syntax near 'guid'

Please show me what is wrong here?

Kieren Johnstone · Answer 1 · 2011-07-14T14:09:06.060

12

"SELECT guid" + "FROM trafficScotland" + "WHERE guid ="

That's:

SELECT guidFROM trafficScotlandWHERE guid =

It makes no sense to break that down into separate strings anyway, but you are missing spaces between words :)

string resultGuidAsString = null;

// build command object
string cmdQuery = "SELECT guid FROM trafficScotland WHERE guid=@guid";
SqlCommand myCmd = new SqlCommand(cmdQuery, myConnection);

// safely pass in GUID parameter value
myCmd.Parameters.AddWithValue("@guid", guid1);

// read result, check for nulls in DB
object result = myCmd.ExecuteScalar();
if (result != DBNull.Value && result != null)
{
    resultGuidAsString = result.ToString();
}

^^ Here's an improved version. Several points for criticism if I may:

No parameters were used for your query: just building one string. A security, readability and maintainability risk
Presumably you're checking whether there is an entry with that guid, suggesting there might not be, but you're not checking for DBNull.Value in case there isn't
Just a bit confusing - you're returning a string but dealing with Guids. Odd.

edited Jul 14 '11 at 14:09

answered Jul 14 '11 at 14:04

Kieren Johnstone

41,277
16
94
144

Not only "makes no sense", but it also forces concatenation, slowing down the routine, as strings are immutable. ;-) – Gregory A Beamer Jul 14 '11 at 14:05
Exactly right, plus you may want to consider using parameterized queries to protect against sql injection. – Thomas Jul 14 '11 at 14:07
1

@Gregory: Concatenating string literals with `+` does _not_ slow down the routine. The C# compiler will do the concatenation at compile time. Thus, you can use `+` to split a long string literal into multiple lines in the source code. – Stephan Jul 14 '11 at 14:27
@Stephan: Thank you for the correction. I guess it is time to go grab some coffee. :-0 – Gregory A Beamer Jul 14 '11 at 14:28
@Kieren your example helped me immensley to learn C# best practice on how to identify if a SQL record does not exist so code can handle that situation appropriately. Thank you! – Doreen Oct 18 '13 at 21:21

Yuck · Answer 2 · 2012-11-13T15:44:16.933

6

Do something like this instead:

var selectString = "SELECT 1 FROM trafficScotland WHERE guid = @guid"
var myCommand = new SqlCommand(selectString, myConnection);
myCommand.Parameters.AddWithValue("@guid", guid1);

var itExists = (Int32)myCommand.ExecuteScalar() > 0;
if (itExists) {
    // do stuff...
}

edited Nov 13 '12 at 15:44

answered Jul 14 '11 at 14:07

Yuck

49,664
13
105
135

score 2 · Accepted Answer · answered Jul 14 '11 at 14:05

2

selectString = "SELECT guid " + "FROM trafficScotland" + " WHERE guid = '" + guid1 +"'";

Notice space after guid

answered Jul 14 '11 at 14:05

hungryMind

6,931
4
29
45

3

-1 Again, you should never ever pass a parameter to a query this way. There's just no good reason for it – Kieren Johnstone Jul 14 '11 at 14:41
2

This is prone to SQL injection. I'd prefer a solution with parameters as proposed in other answers. – Stephan Jul 14 '11 at 14:44

score 2 · Answer 4 · answered Jul 14 '11 at 14:09

Everybody told you what the problem is. Yeah, you's query is not correct. But what about your future queries? How you wanna see if they're right?

I strongly suggest that you use SQL Server Profiler. Profiler sits between your application and the database engine, and grasps every command and query passed to database engine. Thus you can see what's being passed to SQL Server, grab it, and try to execute it in SQL Server Management Studio to debug it.

score 2 · Answer 5 · answered Jul 14 '11 at 14:10

Ideally you'd use parameters to guard against SQL injection. They will also handle things like quoting values that need to be quoted (like GUIDs):

var selectString =  "SELECT guid FROM trafficScotland WHERE guid = @guid";
var myCommand = new SqlCommand(selectString, myConnection);
myCommand.Parameters.AddWithValue("@guid", guid1);
strResult = (String)myCommand.ExecuteScalar();

score 2 · Answer 6 · answered Jul 14 '11 at 14:12

First you have to fix your spacing, you are concatenating part of a query together and you are missing space between vital sql server keyword.

SELECT guidFROM trafficScotlandWHERE guid

Secondly, you should use named parameters. This will help avoiding sql injection and this prevent you from having to think if you need single quote or not around your sql variables.

var query = "SELECT guid FROM trafficScotland WHERE guid = @guid";
using(var command = new SqlCommand(query, connection))
{
    command.Parameters.AddWithValue("@guid", guid1);

    var result = command.ExecuteScalar();

    // Compare guid1 to result
}

score 1 · Answer 7 · answered Jul 14 '11 at 14:04

1

As written, because you forgot some spaces, you are running the following query

SELECT guidFROM trafficScotlandWHERE guid = {guid here}

Proper spacing is the first thing to solve.

answered Jul 14 '11 at 14:04

Gregory A Beamer

16,870
3
25
32

score 1 · Answer 8 · answered Jul 14 '11 at 14:04

1

try this:

string selectString = "SELECT guid FROM trafficScotland WHERE guid = '" + guid1 + "'";

answered Jul 14 '11 at 14:04

danyolgiax

12,798
10
65
116

Thx for your -1! I think this solution is not perfect but is not wrong! You cannot know if he use a Guid.Parse to check for sql injection! – danyolgiax Jul 14 '11 at 14:16
1

My personal policy is to make any option which is a security risk and bad/lazy programming practise have a lower score. Just because its the easiest, laziest route that works doesn't mean it's 'right'. It's the wrong answer in my book – Kieren Johnstone Jul 14 '11 at 14:23
Are you going to downvote only my answer? There are other tree answers in this topic like mine why don't downvote them? I don't want to start a flame but your acting sounds very strange to me! – danyolgiax Jul 14 '11 at 14:29

score 0 · Answer 9 · answered Jul 14 '11 at 14:07

0

whitespace + enclose the guid with "'" as others have mentioned. You should also be storing GUIDs as UNIQUEIDENTIFIERs (assuming MSSQL)

answered Jul 14 '11 at 14:07

Andrew Bullock

36,616
34
155
231

C# checking if record exists in SQL error

9 Answers9