-1

I am trying to Delete a Token from my SQL Database after it has been used.

            MySqlCommand cmdSel = new MySqlCommand("SELECT * FROM tokens WHERE token = " + int.Parse(passbox.Text), dbCon);
            MySqlDataReader dbRead = cmdSel.ExecuteReader();
            if (dbRead.Read())
            {
                int sqlkey = int.Parse(dbRead["token"].ToString()); 
                if (keyint == sqlkey)
                {
                    using (MySqlCommand delTok = new MySqlCommand("DELETE FROM tokens WHERE token = " + keyint, dbCon))
                    {
                        delTok.ExecuteNonQuery(); //MAIN PROBLEM HERE.
                        /*
                        MySql.Data.MySqlClient.MySqlException: 'There is already an open DataReader associated with this Connection which must be closed first.'
                        */
                        //ERROR ^^^^^^
                    }
                    try
                    {
                        dbCon.Close();
                        loading loading = new loading();
                        loading.Show();
                        this.Hide();
                    }
                    catch (Exception ex)
                    {
                        MessageBox.Show(ex.Message);
                        return;
                    }
                }
            }

do i have to close the DataReader or is there some way else, and how do i close the reader? i want to Delete the token once the if keyint is sqlkey statement is true/done. the Error only shows once i try to execute the script for the if statement. The "token" is an int(10)

EngineEye
  • 11
  • 7
  • 2
    The `SELECT` query is completely unnecessary. `DELETE` won't delete something that's not there. Also, you really need to wrap these things in a `using`. What you have is a memory and connection leak. – 3Dave Jan 14 '20 at 02:04
  • why is the SELECT query unnecessary? the DELETE will delete something, i already have data in the DB. how would i use `using`? `using(i put all the MySql commands here)` is this correct? – EngineEye Jan 14 '20 at 02:18
  • 1
    You're saying "Does this row exist? If so, delete it." A delete says "Delete this row if it exists." It's redundant. `using` and `dispose` are already well documented all over the web. Also, don't paste form submission data directly into a query unless you want your site to say "HACKED BY HIGHSCHOOLERS" in a week. – 3Dave Jan 14 '20 at 02:20
  • the tokens are generated on my website automatically, a 10digit int, i am trying to check if the user has used the token, if so, i would log him in, and delete the token from the database since i dont want it to be used more than once. – EngineEye Jan 14 '20 at 02:24
  • 2
    This entire function could be replaced by a single `delete`. `ExecuteNonQuery()` returns the number of rows affected. If it's > 0, then the token existed and was deleted. You don't have to `select` first. – 3Dave Jan 14 '20 at 02:45
  • Why don't use Select Count ? Then delete if the row is > 0 . [Sample](https://stackoverflow.com/questions/59170835/how-do-i-read-hasrow-then-update?answertab=active#tab-top) – Bap mop Jan 14 '20 at 03:40
  • 1
    @Bapmop That is also completely unnecessary. `delete` returns the same thing that `select count` would. The '*' isn't the issue. – 3Dave Jan 14 '20 at 05:02

3 Answers3

2

Connection allows only one open reader.

You can get rid of problem with two datareaders, by executing only one "delete" query.
If token found, query will remove it, if not, query will do nothing. 

using (var connection = new MySqlConnection("connection-string"))
using (var command = connection.CreateCommand())
{
    command.CommandText = "DELETE FROM tokens WHERE token = @token";
    var token = new MySqlParameter
    {
        ParameterName = "@token",
        MySqlDbType = MySqlDbType.Int32,
        Value = int.Parse(passbox.Text)
    };
    command.Parameters.Add(token);

    connection.Open();
    command.ExecuteNonQuery();                
}

Don't try to "keep" connection, simply dispose previous and create new every time you need. ADO.NET in background effectively reuse already opened actual connections.

Use sql parameters for passing values to the query. Sql parameters defend from sql injections and improve sql queries performance, by reusing precompiled query plans.

Fabio
  • 31,528
  • 4
  • 33
  • 72
0

You have to close the reader first in order to execute another command. Also, you can't execute a command while Db reader is reading data. So you can use a function or you can execute the command after the reader is closed.


int sqlkey=0;

using(MySqlCommand cmdSel = new MySqlCommand("SELECT * FROM tokens WHERE token = " + int.Parse(passbox.Text), dbCon))

 {

           MySqlDataReader dbRead = cmdSel.ExecuteReader();
           if (dbRead.Read())
             {
               sqlkey = int.Parse(dbRead["token"].ToString()); 

               }

       reader.close();

  }

      if (keyint == sqlkey)
               {
                   using (MySqlCommand delTok = new MySqlCommand("DELETE FROM tokens WHERE token = " + keyint, dbCon))
                      {

                       delTok.ExecuteNonQuery(); 


                   try
                   {
                       dbCon.Close();
                       loading loading = new loading();
                       loading.Show();
                       this.Hide();
                   }
                   catch (Exception ex)
                   {
                       MessageBox.Show(ex.Message);
                       return;
                   }
                 }
}
UTSHO
  • 113
  • 2
  • 8
-1

Made a new private void and called it with the token after the if statement.

        private void delQuery(int token)
        {
            //SETUP CONNECTION
            MySqlConnection dbConn = new MySqlConnection("some connection");

            //OPEN CONNECTION
            dbConn.Open();

            //DELETE TOKEN
            MySqlCommand delcmd = new MySqlCommand("DELETE FROM tokens WHERE token = " + token, dbConn);
            MySqlDataReader dbReader = delcmd.ExecuteReader();
            dbReader.Read();

            //CLOSE CONNECTION
            dbConn.Close();
        }

called it using:

if (dbRead.Read())
            {
                int sqlkey = int.Parse(dbRead["token"].ToString());
                if (keyint == sqlkey)
                {
                    dbCon.Close();
                    delQuery(keyint);
                }
            }
EngineEye
  • 11
  • 7