ericgrandt / TotalEconomy

All in one economy plugin for Minecraft.
https://ericgrandt.github.io/TotalEconomy/
MIT License
32 stars 33 forks source link

SQL syntax error when creating virtual accounts. #323

Closed jmecn closed 5 years ago

jmecn commented 5 years ago

Sponge Version: 7.1.0-SNAPSHOT Forge Version: 1.12.2-1.12.2-14.23.5.2768 Total Economy Version: 1.8.1 GriefPrevention: 1.12.2-4.3.0.639

Description of issue

I have GriefPrevention and TotalEconomy on my server, and I set GriefPrevention claim.bank-tax-system=true and claim.tax-apply-hour=12. The server crashes very day at 12:00 when GriefPrevention applying claim tax task.

[12:00:00] [Server thread/INFO] [STDERR]: [com.erigitic.sql.SqlQuery:executeQuery:68]: java.sql.SQLSyntaxErrorException: (conn:858) 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 'WHERE uid='bc7fc8ed-f75b-45a0-ad9d-103313551c62'' at line 1
...
[12:00:00] [Server thread/INFO] [STDERR]: [java.lang.Throwable:printStackTrace:634]: Caused by: java.sql.SQLException: 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 'WHERE uid='bc7fc8ed-f75b-45a0-ad9d-103313551c62'' at line 1
Query is: INSERT IGNORE INTO virtual_accounts (dollar_balance) VALUES ('100') WHERE uid='bc7fc8ed-f75b-45a0-ad9d-103313551c62'
...
30 seconds later...
[12:00:30] [Server thread/INFO] [STDERR]: [com.erigitic.sql.SqlQuery:executeQuery:68]: java.sql.SQLTransientConnectionException: HikariPool-1 - Connection is not available, request timed out after 30003ms.

I think this line of SQL is to blame:

 INSERT IGNORE INTO virtual_accounts (dollar_balance) VALUES ('100') WHERE uid='bc7fc8ed-f75b-45a0-ad9d-103313551c62'

Which is builded in AccountManager#createAccountInDatabase(TEVirtualAccount virtualAccount)

Console errors

see https://gist.github.com/jmecn/f8574fe02b55f2ebd3514ebaa724f8d5

Steps to reproduce

set GriefPrevention claim.bank-tax-system=true set GriefPrevention claim.tax-apply-hour to any time. Wait and see the console log.

MarkL4YG commented 5 years ago

Hey there, thank you for your submission. However, I think that just deleting the WHERE clause and adding the uid to the column and value lists would suffice as we are trying to create an account that does not exist anyways. The statement would then simply insert the default balance for all registered currencies.
The issue seems to be that the insert-query uses WHERE instead of just adding the uid column to the column list.
INSERT IGNORE INTO virtual_accounts (uid, dollar_balance) VALUES ('...', 100)

jmecn commented 5 years ago

As I saw in line 160, the virtual_accounts table may have one uid column and multiple columns for different currencies.

create table virtual_account(
    uid varchar(60) NOT NULL,
    dollar_balance decimal(19,2) NOT NULL DEFAULT '100',
    yuan_balance decimal(19,2) NOT NULL DEFAULT '100',
    xxx_balance decimal(19,2) NOT NULL DEFAULT '100'
);

So I think it's better to insert uid once and update for other _balance columns.

update virtual_account set dollar_balance='100', yuan_balance='100' where uid=''

But as the SQL Builder can't support such statement, so I made multiple update statements.

MarkL4YG commented 5 years ago

Sounds reasonable enough to me tbh.
The schema is affected (and replaced) by a new one once the sql refactor is complete too. So I think that any sufficient fix will do the job just fine.

jmecn commented 5 years ago

Another SQL syntax error.

It happened when player set job to farmer, then comes a no column 'farmer' found exception.

I take a look at AccountManager.java, see that experience and levels table do not have farmer column at all.

https://github.com/Erigitic/TotalEconomy/blob/3b31552e753eb8cd67dcb3b3010190252c12974d/src/main/java/com/erigitic/config/AccountManager.java#L164

https://github.com/Erigitic/TotalEconomy/blob/3b31552e753eb8cd67dcb3b3010190252c12974d/src/main/java/com/erigitic/config/AccountManager.java#L172

Solution:

  1. Manually add SQL statement below to your database:
ALTER TABLE `levels` ADD COLUMN `farmer` int(10) unsigned NOT NULL DEFAULT '0';
ALTER TABLE `experience ` ADD COLUMN `farmer` int(10) unsigned NOT NULL DEFAULT '1';
  1. I made an additional fix to last PR.
jmecn commented 5 years ago

This is just a bug fix. In fact I think the table should be like this:

CREATE TABLE `player_jobs` (
    `uid` VARCHAR(60),
    `job` VARCHAR(50) NOT NULL,
    `level` INT(10) unsigned NOT NULL DEFAULT '1',
    `experience ` INT(10) unsigned NOT NULL DEFAULT '0'
)

One may have multiple rows for different job, level and exp.

MarkL4YG commented 5 years ago

@jmecn Yes, that is exacly what I am planning to implement for the refactor.
You can take a look at the schema here

ericgrandt commented 5 years ago

This has been merged.