-1

In my code, am always getting the Value of 'user_id' as -1. is that anything Wrong in the Query???

and i am also want the join query for that.

            using (SqlConnection con = new SqlConnection(connectionString))
            {
                try
                {
                    SqlCommand cmd;
                    if (con.State == ConnectionState.Closed)
                    {
                        con.Open();
                    }
                    var user = Page.User.Identity.Name;
                    cmd = new SqlCommand("select UserId from Users where UserName='"+user+"'", con);
                    var user_id=cmd.ExecuteNonQuery();
                    cmd = new SqlCommand("select Gender from UserDetails where userId='"+user_id+"'");
                    var gender = cmd.ExecuteNonQuery();
                    SqlDataReader rdr = cmd.ExecuteReader();
                    if (rdr.Read())
                    {

                    }
                }
                catch (Exception e) { }
            }
user7415073
  • 290
  • 4
  • 22
  • 2
    Try cmd.ExecuteScalar and cast it to an int or long before assigning to a var. – Joe C Feb 09 '17 at 12:36
  • 1
    You should only use `ExecuteNonQuery` if you don't expect a result. In this case you probably want `ExecuteScalar` instead. - http://stackoverflow.com/questions/2974154/what-is-the-difference-between-executescalar-executereader-and-executenonquery – smoksnes Feb 09 '17 at 12:37
  • 1
    [Return Value Type: System.Int32 The number of rows affected.](https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlcommand.executenonquery(v=vs.110).aspx) – rene Feb 09 '17 at 12:37
  • Check your Where clause whether the username exists within your table of Users or not and also as @JoeC mentioned, use `ExecuteScalar`. – M. Adeel Khalid Feb 09 '17 at 12:37
  • 2
    also use [SqlCommand.Parameters](https://msdn.microsoft.com/en-us//library/system.data.sqlclient.sqlcommand.parameters(v=vs.110).aspx) to prevent sql injection attacks – rene Feb 09 '17 at 12:39
  • @jeyanthi : use ExecuteReader instead of ExecuteNonquery and also provide info what you want to achieve – Chintan Udeshi Feb 09 '17 at 12:43
  • 1
    @SuprabhatBiswal why? What benefit do you think they provide in *this* case? It's not protection from SQL injection, that's achieved by parameterized queries. It's not simplicity, performance or maintainability – Panagiotis Kanavos Feb 09 '17 at 12:47
  • 1
    @jeyanthi you are going good .. But use ExecuteScalar() .As single result is required to return . – UJS Feb 09 '17 at 12:55
  • @PanagiotisKanavos: Apologizes. At first look it seemed OP was trying to return a Dataset, thats why I suggested use of **Stored Procedure**. I checked OP query again and realized **Stored procedure** in not necessary in this case, what OP is trying to achieve is this `select u.UserId, ud.Gender from Users u left join UserDetails ud on u.UserId = ud.userId and u.UserName = '"+user+"'"`. – Suprabhat Biswal Feb 09 '17 at 13:01
  • @SuprabhatBiswal again, why? There is no difference between a query that returns one value and one that returns many. You don't need a stored procedure for this at all. BTW your statement uses string concatenation, allowing SQL Injection attacks. – Panagiotis Kanavos Feb 09 '17 at 13:09
  • @PanagiotisKanavos: Neglect **SQL Injection** stuff this part can be handled with or without use of stored procedure. The thing I tried to convey is why go for two database call using two different queries when same can be achieved in one liner. `select u.UserId, ud.Gender from Users u left join UserDetails ud on u.UserId = ud.userId and u.UserName = @Username` – Suprabhat Biswal Feb 09 '17 at 13:15

4 Answers4

3

You should use ExecuteScalar instead of ExecuteNotQuery.

ExecuteNonQuery:

  1. It will not return any data.
  2. It is used with insert and update.
  3. It returns only the number of rows affected.

ExecuteScalar:

  1. It returns only one value.
  2. That value will the first column first row value.

ExecuteReader

  1. Its for command objects.
  2. It returns the value given by database through select statement.
Mateusz Radny
  • 301
  • 1
  • 6
3

First, ExecuteNonQuery doesn't return any results. If you want to return a single value, use ExecuteScalar.

Second, by concatenating strings to create a SQL statement you are vulnerable to SQL Injection and conversion errors. Google for "Bobby Tables". Or imagine what would happen if someone entered '; DELETE FROM Users;-- as the username.

Third, if you want to retrieve a user's details, don't perform two separate queries. Use a JOIN, eg:

var query = "Select Gender from UserDetails d " +
            " inner Join Users on Users.UserId=d.UserID " + 
            " where UserName=@user";

var genderCmd=new SqlCommand(query);
genderCmd.Parameters.Add("@user",SqlDbType.NVarChar,30);

using (SqlConnection con = new SqlConnection(connectionString))
{
    con.Open();

    genderCmd.Connection=con;
    genderCmd.Parameters["@user].Value=user;
    var gender=(string)genderCmd.ExecuteScalar();
    return gender;
}

You can store the query and command in fields and reuse them as necessary, eg:

void InitializeCommands()
{
    var query = "Select Gender from UserDetails d " +
                " inner Join Users on Users.UserId=d.UserID " + 
                " where UserName=@user";

    _genderCmd=new SqlCommand(query);
    _genderCmd.Parameters.Add("@user",SqlDbType.NVarChar,30);

}

//....

public string GetGender(string user)
{
    using (SqlConnection con = new SqlConnection(connectionString))
    {
        con.Open();

        genderCmd.Connection=con;
        genderCmd.Parameters["@user"].Value=user;
        var gender=(string)genderCmd.ExecuteScalar();
        return gender;
    }
}

You can use the asynchronous versions of Open, ExecuteScalar to avoid blocking a thread while waiting for the server to respond, eg:

public Task<string> GetGender(string user)
{
    using (SqlConnection con = new SqlConnection(connectionString))
    {
        await con.OpenAsync();

        genderCmd.Connection=con;
        genderCmd.Parameters["@user"].Value=user;
        var gender=await genderCmd.ExecuteScalarAsync();
        return (string)gender;
    }
}

IO operations like calls to the database are handled using IO completion ports at the network subsystem level, not threads.

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
0

ExecuteNonQuery does not return anything but the number of affected records for INSERTs, DELETEs or UPDATEs. It returns -1 for all other types of queries.

What you want is ExecuteScalar. Please note that the result of a query can also be NULL (DBNull.Value).

Thorsten Dittmar
  • 55,956
  • 8
  • 91
  • 139
0

AS MSDN says, ExecuteNonQuery returns:

For UPDATE, INSERT, and DELETE statements, the return value is the number of rows affected by the command. When a trigger exists on a table being inserted or updated, the return value includes the number of rows affected by both the insert or update operation and the number of rows affected by the trigger or triggers. For all other types of statements, the return value is -1. If a rollback occurs, the return value is also -1.

In your case, you will need for sure to use ExecuteScalar method, that returns the first value of your query result. You should also use query parameters instead of passing them as text, as it would lead you to have SQL injection

cmd = new SqlCommand("select UserId from Users where UserName= @user, con);
cmd.Parameters.AddWithValue("@user", user);
int user_id = Convert.ToInt32(cmd.ExecuteScalar()); //NOTE THAT YOU WILL ALWAYS HAVE TO CAST THE RESULT, AS IT RETURNS Object
NicoRiff
  • 4,803
  • 3
  • 25
  • 54
  • Note also that you should always check the result for `null`/`DBNull.Value` before you cast. – Thorsten Dittmar Feb 09 '17 at 12:45
  • Bobby tables. String concatenation. SQL Injection. NO. Assume people will copy the answer as is. Create a properly parameterized query, eg `"SELECT ... UserName=@user",con);cmd.Parameters.AddWithValue("@user",user);"`. It's only one more line – Panagiotis Kanavos Feb 09 '17 at 12:47