1

I have problems running this sql statement. It works fine if I run it in mysql but in java I get this error:

You have an error in your SQL syntax; check the manual that corresponds 
  to your MySQL server version for the right syntax to use near '' at line 1

The database has an id(pk) autogenerated, varchar, int, varchar;

Can someone help me?

int i = statement.executeUpdate("INSERT INTO sala values('','"+ nume.getText() + "', "+ Integer.parseInt(capacitate.getText())+ ", '" + sunet.getText()+"'");
4
  • 2
    Please please please read this and take heed: sommarskog.se/dynamic_sql.html#SQL_injection. In general, you're probably best off creating the query in a separate variable, then passing that variable to ExecuteUpdate. That way you can see the whole contents of the statement before you try and run it, and might be able to spot the syntax error. Commented May 18, 2012 at 16:19
  • what are the text that you are passing?? I meant what text is passing in nume.getText(), capacitate.getText() ?? Commented May 18, 2012 at 16:19
  • @FahimParkar those are JTextField Commented May 18, 2012 at 16:24
  • I dont meant that.. what are the text that is passed as input?? ex. nume.getText() will have data as "first input" Commented May 18, 2012 at 16:27

2 Answers 2

5

Don't just try to fix this code by tweaking the SQL as per adarshr's answer. You have a fundamental security problem here which you should fix right now. You're open to SQL injection attacks due to including user data directly in your SQL.

You should use a PreparedStatement, with the parameters declared as placeholders in the SQL, but then given values separately. Exactly how you'll do that will depend on your JDBC provider, but it'll look something like this:

// TODO: Fix the column names, and close the statement in a try/finally block
PreparedStatement pst = conn.prepareStatement(
    "INSERT INTO sala (nume, capacitate, sunet) VALUES (?, ?, ?)");
pst.setString(1, nume.getText());
pst.setInt(2, Integer.parseInt(capacitate.getText()));
pst.setString(3, sunet.getText());
pst.executeUpdate();

Note that if you can get capacite in a way which doesn't require integer parsing, that would be good. Otherwise, consider using NumberFormat which is more locale-friendly. Also note that I've added the column names into the SQL to make this more robust in the face of schema changes.

Sign up to request clarification or add additional context in comments.

1 Comment

I have the try/catch block , did not post it. Will user PreparedStatement. Thanks for teaching me to code the right way!
5

You haven't closed your query.

int i = statement.executeUpdate("INSERT INTO sala values('','"+ nume.getText() + "', "+ Integer.parseInt(capacitate.getText())+ ", '" + sunet.getText()+"')");
                                                                                                                                                          ^

But more than all this, you must use PreparedStatement as Jon suggested below.

5 Comments

... and we still have a SQL injection attack opportunity. This is a bad way of solving the most immediate problem.
@Jon You couldn't wait until I linked your answer, could you? :)
Wow, how sloppy of me, thanks a lot, been looking for this error for about an hour.
@adarshr: The problem is that by showing the insecure way, the OP may well just take it and not fix the bigger problem.
I will use the PreparedStatement, will code the right way. Thanks guys for your time!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.