java - SQL Syntax Error Exception when trying to insert row into table

Question

Hi I am having trouble executing the following function without running into the following exception. I'm not sure why this is happening. I think it might have something to do with the quotes. I am using derby database if it matters.

java.sql.SQLSyntaxErrorException

This is the following code I am trying to execute:

public void addAlbum(Album album) throws IOException, SQLException {
    Properties props = new Properties();
    FileInputStream in = new FileInputStream("database.properties");
    props.load(in);
    in.close();

    props.getProperty("jdbc.drivers");
    String url = props.getProperty("jdbc.url");
    String username = props.getProperty("jdbc.username");
    String password = props.getProperty("jdbc.password");

    Connection connection = DriverManager.getConnection(url, username, password);
    Statement statement = connection.createStatement();
    String sql = null;

    if(album instanceof CDAlbum) {
        CDAlbum cdAlbum = (CDAlbum)album;
        sql = "INSERT INTO MyAlbums VALUES ('CD', '" + cdAlbum.getTitle() + "', '" + cdAlbum.getGenre() + "','" + cdAlbum.getArtist() + "', '" + cdAlbum.getTracks() + "');";
    }
    if(album instanceof DVDAlbum) {
        DVDAlbum dvdAlbum = (DVDAlbum)album;
        sql = "INSERT INTO MyAlbums VALUES ('DVD', '" + dvdAlbum.getTitle() + "', '" + dvdAlbum.getGenre() + "','" + dvdAlbum.getDirector() + "', '" + dvdAlbum.getPlotOutline() + "');";
    }

    statement.executeUpdate(sql);
    System.out.println("Album Added!");

    if(statement != null) {
        statement.close();
    }
    if(connection != null) {
        connection.close();
    }
}

This is the exception:

java.sql.SQLSyntaxErrorException: Syntax error: Encountered "t" at line 2, column 5.
at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(Unknown Source)
at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Unknown Source)
at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(Unknown Source)
at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(Unknown Source)
at org.apache.derby.impl.jdbc.EmbedConnection.handleException(Unknown Source)
at org.apache.derby.impl.jdbc.ConnectionChild.handleException(Unknown Source)
at org.apache.derby.impl.jdbc.EmbedStatement.execute(Unknown Source)
at org.apache.derby.impl.jdbc.EmbedStatement.executeLargeUpdate(Unknown Source)
at org.apache.derby.impl.jdbc.EmbedStatement.executeUpdate(Unknown Source)
at au.edu.uow.CollectionDB.MyCollectionDB.addAlbum(MyCollectionDB.java:194)
at au.edu.uow.Collection.CollectionFactory.loadCollection(CollectionFactory.java:136)
at MyCollection.main(MyCollection.java:18)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:120)
Caused by: ERROR 42X01: Syntax error: Encountered "t" at line 2, column 5.
at org.apache.derby.iapi.error.StandardException.newException(Unknown Source)
at org.apache.derby.iapi.error.StandardException.newException(Unknown Source)
at org.apache.derby.impl.sql.compile.ParserImpl.parseStatementOrSearchCondition(Unknown Source)
at org.apache.derby.impl.sql.compile.ParserImpl.parseStatement(Unknown Source)
at org.apache.derby.impl.sql.GenericStatement.prepMinion(Unknown Source)
at org.apache.derby.impl.sql.GenericStatement.prepare(Unknown Source)
at org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement(Unknown Source)
... 11 more

Answer 1

^{XKCD #327 (http://xkcd.com/327/)}

Use a PreparedStatement!

May I suggest:

try (final PreparedStatement preparedStatement = con.prepareStatement(sql)) {
    if (album instanceof CDAlbum) {
        CDAlbum cdAlbum = (CDAlbum) album;
        preparedStatement.setString(1, "CD");
        preparedStatement.setString(2, cdAlbum.getTitle());
        preparedStatement.setString(3, cdAlbum.getGenre());
        preparedStatement.setString(4, cdAlbum.getArtist());
        preparedStatement.setString(5, cdAlbum.getTracks());
    } else if (album instanceof DVDAlbum) {
        DVDAlbum dvdAlbum = (DVDAlbum) album;
        preparedStatement.setString(1, "DVD");
        preparedStatement.setString(2, dvdAlbum.getTitle());
        preparedStatement.setString(3, dvdAlbum.getGenre());
        preparedStatement.setString(4, dvdAlbum.getDirector());
        preparedStatement.setString(5, dvdAlbum.getPlotOutline());
    }
    dvdAlbum.getPlotOutline();
}

This prevents any possibility of weird values in the data causing the query to fail. Also note that I use a try-with-resources construct, this will always close the resources. Your current code has a memory leak if there is an error in the query - the exception will be thrown and the close() calls with be skipped. You have this issue in many places, when you read the file, when you open the connection, etc...

I have also changed your if...if to an if...else if as I suppose it's unlikely that a CDAlbum would also be a DVDAlbum. A naming note - acronyms in class names are best expressed as words - DvdAlbum rather than DVDAlbum.

Further I would suggest that you learn about method overloading as well as polymorphism. Any use if instanceof in your code is a sure sign of code smell.

Although the whole idea of storing completely disparate data in the same table is a sure sign of design problems. Further, fields like tracks - surely that needs to be another table?!