java.sql.SQLException: Column count doesn't match value count at row 1 Servlets Error or Mysql Error?

Question

try {
    Statement stmt = con.createStatement();
    stmt.executeUpdate("INSERT into emailacc(fname,lname,uname,mail,passwd,passwd2,date,month,year) values('"+fname+","+lname+","+uname+","+mail+","+passwd+","+passwd2+","+date+","+selectMonth+","+year+"')");
    out.println("<h3><font color='green'>Information Added Successfully.</font><br> You Are a registered User now.</h3><br>");
    con.close();
} catch(Exception e) {
    out.println("Exception caught : "+e);   
}

Why is it happening? Last time I did the same but it didn't happen, whats wrong with it?

score 4 · Answer 1 · answered Nov 11 '11 at 14:47

4

Well to start with what's wrong with it is that you're including the values directly into your SQL. Don't do that. Ever. Use a parameterized SQL statement via PreparedStatement, and set the parameter values appropriately. That way you don't need to worry about SQL injection attacks, and it'll also be a lot easier to look at what the actual SQL is, without worrying about where the values come from (or rather, separating those two concerns).

I suspect the immediate problem is that you're not quoting any values, so you've got a SQL statement like a longer version of:

INSERT into Foo(name) VALUES (jon)

rather than

INSERT into Foo(name) VALUES ('jon')

... but using parameterized SQL will fix this anyway, so please don't just change the SQL to include single quotes everywhere.

answered Nov 11 '11 at 14:47

Jon Skeet

1,421,763
867
9,128
9,194

asked **9 mins ago**, answered **6 mins ago** - you're really fast :) – Oleg Mikheev Nov 11 '11 at 14:56
1

I just don't think that you should waste yourself in such primitive questions. I don't like the fact that most reputation on this site is being credited for answering numerous primitive questions. – Oleg Mikheev Nov 12 '11 at 11:47
3

@Oleg: So you *don't* think the answer is "not useful" (as the tooltip describes) - you just take issue with my freedom to answer what I wish to answer? Wow. The net votes for an answer should be based on the answer's content, not on who posted it. You should be aware that neither your downvote nor any other upvotes this answer gets will affect my reputation, due to the way the rep cap works. – Jon Skeet Nov 12 '11 at 12:10
An answer to a primitive question is surely less "useful" than the one given to the advanced one. I'm voting up usefuls answers and voting down not useful. And yes, I understand everything about reputation system. Thanks for your time :) – Oleg Mikheev Nov 12 '11 at 12:27
1

@Oleg: I'm deleting my previous comment now, but I'd just like you to remember the purpose of voting - to indicate the quality of an answer for a specific question. If you really think my answer deserves a downvote *on its own merit* (without any reference to its authorship) that's fine - but in that case, I'd appreciate comments about the *answer* rather than about my choice of which questions to answer. – Jon Skeet Nov 12 '11 at 12:58
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/4929/discussion-between-oleg-and-jon-skeet) – Oleg Mikheev Nov 12 '11 at 13:29
1

@JonSkeet I'm upvoting because the answer is fine, although I would have swapped the answer with the warning. And even though you and Balus pushed me outta #1 ;) – Dave Newton Nov 12 '11 at 15:40

score 1 · Answer 2 · answered Nov 11 '11 at 14:49

1

Your insert statement is missing quotes between the string insert statement should be:

INSERT into emailacc(fname,lname,uname,mail,passwd,passwd2,date,month,year) values('"+fname+"','"+lname+"','"+uname+"','"+mail+"','"+passwd+"','"+passwd2+"',"+date+","+selectMonth+","+year+")");

every column varchar or text should be between single quotes also double check your date format you might have to use the to_date function : to_date(,'DD-MM-YYYY') just a sample

answered Nov 11 '11 at 14:49

Stainedart

1,929
4
32
53

This is a *bad* way of "fixing" the problem. The OP *really* should be using parameterized SQL instead. – Jon Skeet Nov 11 '11 at 14:58
you're right it would be better but we are all missing on a design flaw. The code seems to be written right into a servlet because of the out.println('html code'). This should obviously be extracted to a java class to be precompiled and it would be a much better design "do-it-all" script are not meant to be coded in java – Stainedart Nov 11 '11 at 15:11
Sure, it's not great design either way - but I'd rather have something badly designed but fundamentally secure than a *screaming* security hole which is what your answer still leaves (with no warning). – Jon Skeet Nov 11 '11 at 15:14
I believe there are different ways of fixing sql injection and proper user input validation is one of them. A good regular expression filter will pick them up every time. But I am not one to dictate how people are to proceed. I agree parameterized sql is better. This is just getting _silly_ – Stainedart Nov 11 '11 at 15:36

score 1 · Accepted Answer · answered Nov 11 '11 at 14:56

Is because you are omitting single quotes, for avoid this mistakes my recommendation is to use PreraredStatement, also in order to proper close connection it mus be in a finally block , you code must look at this:

    try {
        PreparedStatement stmt = con.prepareStatement("INSERT into emailacc(fname,lname,uname,mail,passwd,passwd2,date,month,year) values(?,?,?,?,?,?,?,?,?)");
        stmt.setString(1,fname);
        stmt.setString(2,lname);
        stmt.setString(3,uname);
        stmt.setString(4,mail);
        stmt.setString(5,passwd);
        stmt.setString(6,passwd2);
        stmt.setDate(7,date); //you need convert your date to java.sql.Date if 'date' field of database is of type date. If not setString is fine
        stmt.setInt(8,selectMonth);
        stmt.setInt(9,year);
        stmt.executeUpdate();
        out.println("<h3><font color='green'>Information Added Successfully.</font><br> You Are a registered User now.</h3><br>");
    } catch (Exception e) {
        con.rollback();
        out.println("Exception caught : " + e);
    } finally {
        if (con != null) {
            try {
                con.close();
            } catch(SQLException ex){
                //DO NOTHING
            }
        }
    }

You can learn more of PreparedStatemt in:

http://download.oracle.com/javase/tutorial/jdbc/basics/prepared.html

A final note: PreparedStament are more efficent thant Statement and avoid the SQL Injection hack so PrepararedStatement is more secure. Try use always a PreparedStatement

java.sql.SQLException: Column count doesn't match value count at row 1 Servlets Error or Mysql Error?

3 Answers3