0

I am new to delphi( or any coding for that matter) and i have been learning SQL for the past week but this code that i have tried using to find and delete a record from a Msaccess database doesn't work. It doesn't give any error when i run it, it just doesn't do anything when i click the button, It displays the message but it doesn't delete the record from the table. I have been using this code:

begin
ADOQuery1.SQL.Text := 'SELECT * FROM Admins WHERE Name = '''+Edtname.text+'''';
ADOQuery1.Open;
if ADOQuery1.IsEmpty then
ShowMessage('User not found')
else
begin
ADOQuery1.Close;
ADOQuery1.SQL.Text := 'DELETE FROM Admins WHERE Name = '''+EdtName.Text+'''';
ADOQuery1.ExecSQL;
ShowMessage('Information was Deleted');
end;
ADOquery1.Free;
end;

Information about the database:

Collumns     Type
========     ====
Name         Text
Surname      Text
Dateadded    Date/time
Password     Text
Adminnumber  Number

Please give as much information as possible as to why this error occured, as I said im still learning and thank you in advance :)

user2748631
  • 15
  • 2
  • 6
  • 3
    I just gave you a very long, complete answer to your [other question] and explained (in detail) why you should *NOT* use string concatenation for building SQL statements, and the very first thing I notice here is two SQL statements that use concatenation. Nice to see I wasted my time. :-( – Ken White Sep 05 '13 at 11:04
  • Yes sorry but i will try to learn the other ways of building an sql statement but this is the code that is the easiest way to start SQl's in according to my textbook and i have gotten it to work... – user2748631 Sep 05 '13 at 12:46
  • And also the worst way... One day while you'll be writing more complex queries, you get lost in quotes. Look, here is a next [`today's example`](http://stackoverflow.com/q/18636311/960757) of SQL query concatenation problem. – TLama Sep 05 '13 at 12:57
  • 1
    As I mentioned in my previous answer, teaching yourself the **proper** way from the start will mean you have much fewer headaches (and learn much more in the process). If you're not going to learn from the answers to your questions, it would probably be better if you just stuck to the book and figured it out yourself. :-) – Ken White Sep 05 '13 at 13:05

2 Answers2

3

If you insist on using string concatenation (despite all advice to the contrary), at least eliminate the noise of trying to count the single quotes and use QuotedStr (once again, the link is to XE4 documentation, but the function exists in Delphi 7 as well).

The same information I provided in my answer to your other question also applies here. Name is still a reserved word in MS Access, and therefore still needs to be surrounded by []. It will need that every single time you use it, which is also why I suggested you change the field name before you got too far along.

The code you posted shows the ADOQuery being freed at the end, but does not show it being created. I've added that code so that it makes sense; you'll need to replace the connection string with one to your database. I've also changed the name of the ADOQuery from ADOQuery1 (which will conflict with any existing ADOQuery on your form with the default name) because your code appears to be creating a new one for just this block of code. If in fact you're using one already on the form or datamodule, you should delete the try, Create, ConnectionString, and lines finally, Free, and the next end, and rename all of the TempQuery variables back to ADOQuery1.

var
  NumRows: Integer;
  TempQuery: TADOQuery;
begin
  TempQry := TADOQuery.Create(nil);
  try
    TempQuery.ConnectionString := 'Use your own connection string here';

    TempQuery.SQL.Text := 'SELECT * FROM Admins WHERE [Name] = ' +
                           QuotedStr(Edtname.text);
    TempQuery.Open;
    if TempQuery.IsEmpty then
    begin
      ShowMessage('User ' + EdtName.Text + ' not found!');
      Exit;
    end;

    TempQuery.Close;
    TempQuery.SQL.Text := 'DELETE FROM Admins WHERE [Name] = ' +
                           QuotedStr(EdtName.Text);
    TempQuery.ExecSQL;
    NumRows := TempQuery.RowsAffected;
    ShowMessage(IntToStr(NumRows) + ' were deleted');
  finally
    TempQuery.Free;
  end;
end;

Once again, however, this would be better using parameterized queries. It only adds two additional lines of code, and eliminates the security risks involved with SQL injection at the ExecSQL line:

TempQuery.SQL.Text := 'SELECT * FROM Admins WHERE [Name] = :UserName';
TempQuery.Parameters.ParamByName('UserName').Value := EdtName.Text;
TempQuery.Open;

TempQuery.SQL.Text := 'DELETE FROM Admins WHERE [Name] = :UserName';
TempQuery.Parameters.ParamByName('UserName').Value := EdtName.Text;
TempQuery.ExecSQL;
Ken White
  • 123,280
  • 14
  • 225
  • 444
  • 1
    +1 for advocating the use of parameters, maybe you can include a link to wikipedia about SQL injection? http://en.wikipedia.org/wiki/SQL_injection – whosrdaddy Sep 05 '13 at 13:54
  • @whosrdaddy: I'll also add it to my [previous answer](http://stackoverflow.com/a/18625952/62576) to this same poster, where I gave a lecture in depth about the risk of SQL injection and suggested a search here or at Google (and was ignored). See my first sentence in this answer, as well. :-) Thanks for the link; it saved me the trouble of locating it. – Ken White Sep 05 '13 at 14:33
  • 1
    @TLama: Thanks for the edit. Must have been a sloppy copy/paste/edit on my part. – Ken White Sep 05 '13 at 14:35
0

Have you set the connection string for the ADOQuery? you can check the number of rows affected by the query by assigning an integer to ADOQuery1.ExecSQL: http://docs.embarcadero.com/products/rad_studio/delphiAndcpp2009/HelpUpdate2/EN/html/delphivclwin32/ADODB_TADOQuery_ExecSQL.html

When you say the message is displaying - which message? are there any error details?

Davie Brown
  • 717
  • 4
  • 23