MySQL value not updating

Discussion in 'Spigot Plugin Development' started by DotRar, Jun 14, 2016.

  1. So I have this method:

    Code (Text):
    public void setSeconds(final String name, final int seconds) {
            String toExecute = "INSERT INTO doubleCoins(`NAME`, `SECONDS`) VALUES('"+name+"', '"+seconds+"') ON DUPLICATE KEY UPDATE `SECONDS` = '"+seconds+"';";
            try  {
                PreparedStatement ps = conn.prepareStatement(toExecute);
                ps.execute();
                ps.close();
            }
            catch(SQLException e) {
                e.printStackTrace();
            }
        }
    Which works fine in some places, but not in others.

    Now in my onDisable method, I have this:

    Code (Text):
    @Override
        public void onDisable() {
            db.setSeconds(Timer.INSTANCE.name, Timer.INSTANCE.seconds);
            System.out.println("1:"+Timer.INSTANCE.seconds);
            System.out.println("2:"+db.getSeconds());
            db.closeConnection();
        }
    My console output looks like this though:
    Code (Text):
    1:3594
    2:3600
    And when the server boots up again, the timer starts counting down from 3600.

    Any ideas to why it isn't setting the value?
     
  2. Just found out from testing, if there are 2 boosters, the active one saves fine.
     
  3. Could it be that you're wrapping your integers in single quotes? If removing the quotes doesn't work, you should post your get method.
     
  4. Nice catch! Will try that
     
  5. Ok I removed the quotes. Here's my updated code w/ the get method:

    Code (Text):
    public int getSeconds() {
            try  {
                PreparedStatement ps = conn.prepareStatement("SELECT * FROM doubleCoins;");
                ResultSet resultSet = ps.executeQuery();
                int toReturn = 0;

                if(resultSet.first()) {
                    try {
                        toReturn =  Integer.parseInt(resultSet.getString("SECONDS"));
                    } catch(NumberFormatException e) {
                        toReturn = 0;
                    }
                }

                ps.close();
                return toReturn;
            }
            catch(SQLException e) {
                return 0;
            }
        }

        public void setSeconds(final String name, final int seconds) {
            String toExecute = "INSERT INTO doubleCoins(`NAME`, `SECONDS`) VALUES('"+name+"', "+seconds+") ON DUPLICATE KEY UPDATE SECONDS = "+seconds+";";
            try  {
                PreparedStatement ps = conn.prepareStatement(toExecute);
                ps.execute();
                ps.close();
            }
            catch(SQLException e) {
                e.printStackTrace();
            }
        }
     
  6. I have three more ideas you could try:

    1) Your setter has a name parameter, but your getter doesn't, so who's booster is it getting? The answer is everyone's. Add a name parameter. Then, in your getter method change "SELECT * FROM doubleCoins;" to
    "SELECT * FROM doubleCoins WHERE name = "+name+;"
    I think what is happening is when you have more than one entry in the doubleCoins table, calling ResultSet.getString() returns comma separated values which Integer.parseInt() can't turn into a single number.

    2) When you catch NumberFormatException print to console so you know when it isn't working, and when you catch SQLException, print the stacktrace.

    3) You have to make sure that the column in the database, the value you set seconds to, and the value of seconds you get are all of the same type (integer or string). I think always using a string for interacting with the database is the best way to go in your case.
    To do this:
    • Run "ALTERTABLE doubleCoins MODIFY SECONDS varchar(255);"
    • Add the quotes back to your setter method

    If that doesn't work, try to learn from the exceptions I suggest you catch from idea #2. If you still can't figure it out, you will have to post more code. However, I strongly suspect your problem can be easily fixed by adding a WHERE conditional to your getter method.
     
  7. Ok something got it working however it justs adds a new row every time. One solution would be making NAME unique however I want 1 player to be able to queue up boosters.
     
  8. I don't use MySQL, so I wouldn't know the best design if you want to have a queue, but if I were in your position I would create two static variables called highestBoosterID and currentBoosterID (per player with hashmaps or globally, depending on if you want the booster to apply to the player only or to everyone online). I would also add an "id" column to your table, so that when the current booster expires you can increment currentBoosterID and SELECT the next booster WHERE id = currentBoosterID. When you want to add a booster, you can increment highestBoosterID and put that in the database.
     
  9. Please close your resources (Statement, ResultSet, Connection) in the finally clause or use try-with-resources.
     
  10. He probably does. You don't see him opening the connection, which means he (almost certainly) uses a global or global-per-player connection that he closes when the server shuts down or when the player logs off.

    EDIT: You should close your PreparedStatement when you catch SQLException. As for ResultSet, Oracle's javadocs say this: "A ResultSet object is automatically closed when the Statement object that generated it is closed, re-executed, or used to retrieve the next result from a sequence of multiple results." It might be best practice to close your ResultSet, but it's not necessary.
     
    #11 LactemSenior, Jun 15, 2016
    Last edited: Jun 15, 2016
  11. My post was more like about where (or how) the OP should close the resources and not which ones (of course that depends on the case). He does it in the try clause but it should be in the finally clause and if an exception occurs also the ResultSet wouldn't be closed in his current code. That means that closing at least the Statement in the finally clause is necessary. You are right with the Connection object but as I have already said that was actually not my intention.
     
    • Don't concatenate values in your query, use the ? placeholder with the set<Type> methods.
    • Learn to design databases, don't do random shit like setting your double columns to be VARCHAR(255).
    • Make sure your name column is UNIQUE, otherwise ON DUPLICATE KEY won't work.
      • If you want every (player, name) pair to be unique, well, add a constraint on both of them at the same time (no, not UNIQUE on both columns!)
    • Stop clinging to static, and encapsulate your fields. Timer.INSTANCE is terrible. (And I bet you got more bad static usage where that came from)
     
  12. I think you're misunderstanding the situation a little. You have a good point about ON DUPLICATE KEY; I had forgotten about that since I don't really use SQL. But the column "doubleCoins" is not a double; it's either a string or an integer that stores the time left for the player to have double coins activated. Additionally, making the name unique will not allow for a queue of boosters.
    I had no idea what it means to "add a constraint," so I looked it up and it looks like an sql-specific way to implement my suggestion of attaching an id to each booster. Yours would be a little cleaner than my way if @Nex9 knows how to do it.
     
  13. Can you give me a place to learn about how I would implement this constraint thing? The best I can find is
     
  14. yea, misread the table. Seconds, by nature, is numeric, and should be an (BIG)INT, not a VARCHAR. As for the uniqueness, I assumed the name was an identifier for a kind of booster (since it's lacking any form of identity otherwise, which would make queueing impossible). Adding a unique constraint on that identifier and a player identifier (which would be an UUID since MC 1.7.5) will sort that out.

    Google will serve you well. Searching "sql unique two columns" would give you your answer in this case.
     
  15. So If I just add an auto increment id column and adjust my code that will allow queueing?
     
  16. First off, what is the name column exactly? Is that the player name? Or the a booster name?
     
  17. Just a numeric identifier