SQL PreparedStatement; Am I doing it right?

Question

I am building a web app with a single (not pooled) full time (jdbc) connection between static classes and the database. This is expected to be a low traffic site and the static methods are synchronized. In an effort to speed up access to product information, I am trying PreparedStatements for the first time. As I test, on localhost, sure that I'm the only one running the app., it seems clear to me that my the prepared statements are slower than the unprepared statements I use earlier in the process. This may not be a fair comparison. The unprepared statements get a single result set from one table and it's done. As you can see from the code below, what I'm doing with prepared statements involves three tables and multiple queries. But since this is my first time, I would appreciate review and comment. This does actually work; i.e. all data is retrieved as expected.

The first method below (initialize()) is called once from a servlet init() method when the application is first started. The second method (getItemBatch()) retrieves information about as many product items as match a product name (Titel). My little development / test database has less than 100 (total) items and only 1-3 items matching each name; most often only 1. The server app and database are on the same machine and I'm accessing from a browser via localhost. I'm surprised by the consistent perceptible wait for this detailed product data compared to fetching a master product list (all items) mentioned above.

 public static boolean initialize (Connection connArg, String dbNameArg, String dbmsArg) {

    con = connArg;
    dbName = dbNameArg;
    dbms = dbmsArg;
    sqlserver_con = connArg;

    ItemBatchStatement =
          "select Cartnr, Hgrupp, Prisgrupp, Moms from dbo.Centralartregister " +
           "where Titel = ?";

    ForArtNrStatement =
          "select Artnr, Antal from dbo.Artikelregister " +
           "where Cartnr = ? and Antal > 0";

    ItemHgruppStatement =
            "select Namn from dbo.Huvudgrupper " +
             "where Hgrupp = ?";

    try {
      con.setAutoCommit(false);
      getItemBatch = con.prepareStatement(ItemBatchStatement);
      getForArtNr = con.prepareStatement(ForArtNrStatement);
      getItemHgrupp = con.prepareStatement(ItemHgruppStatement);
    } catch (SQLException e) {
      return(false);
    } finally {
      try {con.setAutoCommit(true);} catch (SQLException e) {}
    }
    return(true);
  }

-

public static synchronized String getItemBatch (String Titel) throws SQLException {

    String ret_xml;
    ResultSet rs  = null;
    ResultSet rs1 = null;
    ResultSet rs2 = null;

    Titel = charChange(Titel);
    ret_xml = "<ItemBatch Titel='" + Titel + "'>";
    try {
      con.setAutoCommit(false);
      getItemBatch.setString(1,Titel);
      rs = getItemBatch.executeQuery();
      while (rs.next()) {
        getForArtNr.setInt(1,rs.getInt("Cartnr"));
        rs1 = getForArtNr.executeQuery();
        getItemHgrupp.setInt(1,rs.getInt("Hgrupp"));
        rs2 = getItemHgrupp.executeQuery();
        if (rs1.next() && rs2.next()) {
          ret_xml += "<item><Hgrupp>" + rs2.getString("Namn") + "</Hgrupp><Price>" + rs.getInt("Prisgrupp") + "</Price><Moms>" + rs.getInt("Moms") + "</Moms><Cartnr>" + rs.getInt("Cartnr") + "</Cartnr><Artnr>" + rs1.getInt("Artnr") + "</Artnr></item>";
        }
      }
      ret_xml += "</ItemBatch>";
      return(ret_xml);
    } catch (SQLException e) {
      return("SQLException: " + e);
    } finally {
      try {con.setAutoCommit(true);} catch (SQLException e) {}
      if (rs != null) {rs.close();}
      if (rs1 != null) {rs1.close();}
      if (rs2 != null) {rs2.close();}
    }
  }

.

UPDATE: I'm still googling and looking for ways to do better. Let me add something for your consideration.

I'm closing the prepared statements via the servlet's destroy() method; the same servlet that calls the method "initialize()" above in order to create them. The idea is to create them once (and only once) and have them available forever for all users (i.e. until the app or server is shut down). I'm going for a kind-of a pseudo-stored procedure. I would just use stored procedures straight out, but the database exists to support another application (their in-house sales system) and I'm going to use it for Internet sales read-only ... avoiding any potential conflict with their maintenance efforts or agreements etc. by not doing anything to change their database set-up. I have suggested that my app use a new account limited to read-only privileges. And come to think of it, I haven't tested whether I can use Prepared Statements in that mode; just seems like it should work.

Each result set is closed in the finally block in the method where they are created. I guess that's ok? I reuse the same RS variable name for multiple result sets (on the second two) but close only once. But wait! Do I even need that? The ResultSets are declared within the scope of the method. If resetting them without closing the old ones doesn't cause leaks, then exiting the method should do just as well on its own. It is a static method, but the ResultSets will be reset every time it's used (at the very least). So, at most, there would just be a single set of ResultSet handles available for reuse; not run-away "leakage."

I'm wondering if I can send the two select requests in the loop both at the same time, simply by turning them into one prepared statement separated by ';'. Or just found "MultipleActiveResultSets=True"; document allowing SQL Server to process multiple transaction requests on a single connection ... still investigating this. Or is there another way to create a prepared statement that would fetch ALL the data via a single submission? (Seems to me like there are too many round trips.) Finally, I might get a boost from using connection pooling, which I haven't done yet. It's low priority in my project right now, but I might have to do it before we go online.

Answer 1

There are a couple of issues I think you should be aware of.

• In JDBC, the default state of a connection is getAutoCommit() == true.

  you keep changing this to be false, then running some selects, and then setting it back to true.

  if it was true when you came in, then you set it to false. No problem, you then return it to true when you leave... but, if it was false, you keep it as false, then, when you leave, you commit the transaction that may have been open when your method was called.... this is probably not what you want.

  It is possible that there are other areas in your code where you are doing data changing statements, but, it is not apparent where. In your code, all the SQL statements are selects, and you are not changing anything, so your you are not doing any fancy locking. Why change the state of autoCommit at all?

• you are not closing your ResultSets.

  you have a very nice, neat final block which closes rs, rs1, and rs2, but, there are many, many rs1 and rs2 instances. You need to be closing them inside the inner rs loop:

    rs = getItemBatch.executeQuery();
    while (rs.next()) {
      getForArtNr.setInt(1,rs.getInt("Cartnr"));
      try {
          rs1 = getForArtNr.executeQuery();
          getItemHgrupp.setInt(1,rs.getInt("Hgrupp"));
          rs2 = getItemHgrupp.executeQuery();
          if (rs1.next() && rs2.next()) {
            ret_xml += "<item><Hgrupp>" + rs2.getString("Namn") + "</Hgrupp><Price>" + rs.getInt("Prisgrupp") + "</Price><Moms>" + rs.getInt("Moms") + "</Moms><Cartnr>" + rs.getInt("Cartnr") + "</Cartnr><Artnr>" + rs1.getInt("Artnr") + "</Artnr></item>";
          }
      } finally {
          if (rs1 != null) {
              rs1.close();
          }
          rs1 = null;
          if (rs2 != null) {
              rs2.close();
          }
          rs2 = null;
      }
  
    }
  

Answer 2

Yes, you're doing it right. The performance differences you're seeing are most likely due, as you noted, to the differences between your data volumes. Since you're using SQL Server, you can use examine the database trace information to see how the queries perform when you have similar data volumes. Or you can just relax and smile smugly, knowing that your code will never be hacked by a SQL-injection exploit :-)