Upstream/Plan
1
0
Fork 0
mirror of https://github.com/plan-player-analytics/Plan.git synced 2024-06-25 22:24:47 +02:00
Code Issues Projects Releases Wiki Activity

Fixed issues related to MySQL ONLY_FULL_GROUP_BY

Browse Source 
MySQL breaks GROUP BY syntax of standard SQL, and allows arbitrary columns with aggregate functions.
The ONLY_FULL_GROUP_BY stops this, instead raising an error. Since the SQL was originally designed
with this mode on, restricting the SQL broke the code in a few places.

Adding the extra group by clauses solves the issue without effecting query results.

These issues will be caught by MySQLTest in the future, since the issues could be reproduced by
enabling ONLY_FULL_GROUP_BY mode.

Affects issues:
- Fixed #2619
This commit is contained in:
Aurora Lahtela 2022-09-25 10:30:13 +03:00
parent 5c9e0deed4
commit 0a14d17d7a
10 changed files with 71 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Plan/common/src/main/java/com/djrapitops/plan/extension/implementation/storage/queries/ExtensionServerTableDataQuery.java
View File

@ -79,7 +79,7 @@ public class ExtensionServerTableDataQuery implements Query<Map<UUID, ExtensionT
UsersTable.USER_UUID + ",MAX(" + SessionsTable.SESSION_END + ") as last_seen" +
FROM + SessionsTable.TABLE_NAME +
INNER_JOIN + UsersTable.TABLE_NAME + " u on u." + UsersTable.ID + '=' + SessionsTable.TABLE_NAME + '.' + SessionsTable.USER_ID +
GROUP_BY + SessionsTable.TABLE_NAME + '.' + SessionsTable.USER_ID +
GROUP_BY + UsersTable.USER_UUID +
ORDER_BY + "last_seen DESC LIMIT ?";
String sql = SELECT +
@ -123,10 +123,11 @@ public class ExtensionServerTableDataQuery implements Query<Map<UUID, ExtensionT
private Query<Map<UUID, ExtensionTabData>> fetchPlayerGroups() {
String selectLimitedNumberOfPlayerUUIDsByLastSeenDate = SELECT +
UsersTable.USER_UUID + ",MAX(" + SessionsTable.SESSION_END + ") as last_seen" +
UsersTable.USER_UUID + "," +
"MAX(" + SessionsTable.SESSION_END + ") as last_seen" +
FROM + SessionsTable.TABLE_NAME +
INNER_JOIN + UsersTable.TABLE_NAME + " u on u." + UsersTable.ID + '=' + SessionsTable.TABLE_NAME + '.' + SessionsTable.USER_ID +
GROUP_BY + SessionsTable.TABLE_NAME + '.' + SessionsTable.USER_ID +
GROUP_BY + UsersTable.USER_UUID +
ORDER_BY + "last_seen DESC LIMIT ?";
String sql = SELECT +

6
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/PerServerAggregateQueries.java
View File

@ -57,7 +57,7 @@ public class PerServerAggregateQueries {
FROM + SessionsTable.TABLE_NAME +
INNER_JOIN + ServerTable.TABLE_NAME + " se on se." + ServerTable.ID + '=' + SessionsTable.TABLE_NAME + '.' + SessionsTable.SERVER_ID +
WHERE + SessionsTable.USER_ID + "=" + UsersTable.SELECT_USER_ID +
GROUP_BY + SessionsTable.SERVER_ID;
GROUP_BY + ServerTable.SERVER_UUID;
return new QueryStatement<>(sql) {
@Override
public void prepare(PreparedStatement statement) throws SQLException {
@ -103,7 +103,7 @@ public class PerServerAggregateQueries {
FROM + SessionsTable.TABLE_NAME +
INNER_JOIN + ServerTable.TABLE_NAME + " se on se." + ServerTable.ID + '=' + SessionsTable.TABLE_NAME + '.' + SessionsTable.SERVER_ID +
WHERE + SessionsTable.USER_ID + "=" + UsersTable.SELECT_USER_ID +
GROUP_BY + SessionsTable.SERVER_ID;
GROUP_BY + ServerTable.SERVER_UUID;
return getQueryForCountOf(playerUUID, sql, "kill_count");
}
@ -113,7 +113,7 @@ public class PerServerAggregateQueries {
FROM + SessionsTable.TABLE_NAME +
INNER_JOIN + ServerTable.TABLE_NAME + " se on se." + ServerTable.ID + '=' + SessionsTable.TABLE_NAME + '.' + SessionsTable.SERVER_ID +
WHERE + SessionsTable.USER_ID + "=" + UsersTable.SELECT_USER_ID +
GROUP_BY + SessionsTable.SERVER_ID;
GROUP_BY + ServerTable.SERVER_UUID;
return getQueryForCountOf(playerUUID, sql, "death_count");
}

2
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/ServerAggregateQueries.java
View File

@ -89,7 +89,7 @@ public class ServerAggregateQueries {
String sql = SELECT + "COUNT(1) as c, " + ServerTable.SERVER_UUID +
FROM + UserInfoTable.TABLE_NAME +
INNER_JOIN + ServerTable.TABLE_NAME + " s on s." + ServerTable.ID + '=' + UserInfoTable.TABLE_NAME + '.' + UserInfoTable.SERVER_ID +
GROUP_BY + UserInfoTable.SERVER_ID;
GROUP_BY + ServerTable.SERVER_UUID;
return new QueryAllStatement<>(sql, 100) {
@Override

29
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/analysis/ActivityIndexQueries.java
View File

@ -82,8 +82,8 @@ public class ActivityIndexQueries {
public static String selectActivityIndexSQL() {
String selectActivePlaytimeSQL = SELECT +
"ux." + UserInfoTable.USER_ID + ",COALESCE(active_playtime,0) AS active_playtime" +
FROM + UserInfoTable.TABLE_NAME + " ux" +
"ax_ux." + UserInfoTable.USER_ID + ",COALESCE(active_playtime,0) AS active_playtime" +
FROM + UserInfoTable.TABLE_NAME + " ax_ux" +
LEFT_JOIN + '(' + SELECT + SessionsTable.USER_ID +
",SUM(" + SessionsTable.SESSION_END + '-' + SessionsTable.SESSION_START + '-' + SessionsTable.AFK_TIME + ") as active_playtime" +
FROM + SessionsTable.TABLE_NAME +
@ -91,17 +91,17 @@ public class ActivityIndexQueries {
AND + SessionsTable.SESSION_END + ">=?" +
AND + SessionsTable.SESSION_START + "<=?" +
GROUP_BY + SessionsTable.USER_ID +
") sx on sx." + SessionsTable.USER_ID + "=ux." + UserInfoTable.USER_ID;
") ax_sx on ax_sx." + SessionsTable.USER_ID + "=ax_ux." + UserInfoTable.USER_ID;
String selectThreeWeeks = selectActivePlaytimeSQL + UNION_ALL + selectActivePlaytimeSQL + UNION_ALL + selectActivePlaytimeSQL;
return SELECT +
"5.0 - 5.0 * AVG(1.0 / (?/2.0 * (q1.active_playtime*1.0/?) +1.0)) as activity_index," +
"u." + UsersTable.ID + " as " + SessionsTable.USER_ID + ',' +
"u." + UsersTable.USER_UUID +
FROM + '(' + selectThreeWeeks + ") q1" +
INNER_JOIN + UsersTable.TABLE_NAME + " u on u." + UsersTable.ID + "=q1." + UserInfoTable.USER_ID +
GROUP_BY + "q1." + UserInfoTable.USER_ID;
"5.0 - 5.0 * AVG(1.0 / (?/2.0 * (ax_q1.active_playtime*1.0/?) +1.0)) as activity_index," +
"ax_u." + UsersTable.ID + " as user_id," +
"ax_u." + UsersTable.USER_UUID +
FROM + '(' + selectThreeWeeks + ") ax_q1" +
INNER_JOIN + UsersTable.TABLE_NAME + " ax_u on ax_u." + UsersTable.ID + "=ax_q1." + UserInfoTable.USER_ID +
GROUP_BY + "ax_u." + UsersTable.ID + ",ax_u." + UsersTable.USER_UUID;
}
public static void setSelectActivityIndexSQLParameters(PreparedStatement statement, int index, long playtimeThreshold, ServerUUID serverUUID, long date) throws SQLException {
@ -122,13 +122,16 @@ public class ActivityIndexQueries {
public static Query<Integer> fetchActivityGroupCount(long date, ServerUUID serverUUID, long playtimeThreshold, double above, double below) {
String selectActivityIndex = selectActivityIndexSQL();
String selectCount = SELECT + "COUNT(1) as count, COALESCE(activity_index, 0) as activity_index" +
String selectIndexes = SELECT + "COALESCE(activity_index, 0) as activity_index" +
FROM + UserInfoTable.TABLE_NAME + " u" +
LEFT_JOIN + '(' + selectActivityIndex + ") q2 on q2." + SessionsTable.USER_ID + "=u." + UserInfoTable.USER_ID +
WHERE + "u." + UserInfoTable.SERVER_ID + "=" + ServerTable.SELECT_SERVER_ID +
AND + "u." + UserInfoTable.REGISTERED + "<=?" +
AND + "activity_index>=?" +
AND + "activity_index<?";
AND + "u." + UserInfoTable.REGISTERED + "<=?";
String selectCount = SELECT + "COUNT(1) as count" +
FROM + '(' + selectIndexes + ") i" +
WHERE + "i.activity_index>=?" +
AND + "i.activity_index<?";
return new QueryStatement<>(selectCount) {
@Override

20
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/analysis/NetworkActivityIndexQueries.java
View File

@ -85,10 +85,10 @@ public class NetworkActivityIndexQueries {
WHERE + ServerTable.SERVER_UUID + " IN ('" + new TextStringBuilder().appendWithSeparators(onServers, "','") + "')";
String selectActivePlaytimeSQL = SELECT +
"ux." + UsersTable.ID + "," +
"ux." + UsersTable.USER_UUID + "," +
"ax_ux." + UsersTable.ID + "," +
"ax_ux." + UsersTable.USER_UUID + "," +
"COALESCE(active_playtime,0) AS active_playtime" +
FROM + UsersTable.TABLE_NAME + " ux" +
FROM + UsersTable.TABLE_NAME + " ax_ux" +
LEFT_JOIN + '(' + SELECT + SessionsTable.USER_ID +
",SUM(" + SessionsTable.SESSION_END + '-' + SessionsTable.SESSION_START + '-' + SessionsTable.AFK_TIME + ") as active_playtime" +
FROM + SessionsTable.TABLE_NAME +
@ -96,17 +96,17 @@ public class NetworkActivityIndexQueries {
AND + SessionsTable.SESSION_START + "<=?" +
(onServers.isEmpty() ? "" : AND + SessionsTable.SERVER_ID + " IN (" + selectServerIds + ")") +
GROUP_BY + SessionsTable.USER_ID +
") sx on sx." + SessionsTable.USER_ID + "=ux." + UsersTable.ID;
") ax_sx on ax_sx." + SessionsTable.USER_ID + "=ax_ux." + UsersTable.ID;
String selectThreeWeeks = selectActivePlaytimeSQL + UNION_ALL + selectActivePlaytimeSQL + UNION_ALL + selectActivePlaytimeSQL;
return SELECT +
"5.0 - 5.0 * AVG(1.0 / (?/2.0 * (q1.active_playtime*1.0/?) +1.0)) as activity_index," +
"u." + UsersTable.ID + " as " + SessionsTable.USER_ID + ',' +
"u." + UsersTable.USER_UUID +
FROM + '(' + selectThreeWeeks + ") q1" +
INNER_JOIN + UsersTable.TABLE_NAME + " u on u." + UsersTable.ID + "=q1." + UsersTable.ID +
GROUP_BY + "q1." + UsersTable.ID;
"5.0 - 5.0 * AVG(1.0 / (?/2.0 * (ax_q1.active_playtime*1.0/?) +1.0)) as activity_index," +
"ax_u." + UsersTable.ID + " as user_id," +
"ax_u." + UsersTable.USER_UUID +
FROM + '(' + selectThreeWeeks + ") ax_q1" +
INNER_JOIN + UsersTable.TABLE_NAME + " ax_u on ax_u." + UsersTable.ID + "=ax_q1." + UsersTable.ID +
GROUP_BY + "ax_u." + UsersTable.ID + ",ax_u." + UsersTable.USER_UUID;
}
public static void setSelectActivityIndexSQLParameters(PreparedStatement statement, int index, long playtimeThreshold, long date) throws SQLException {

2
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/objects/SessionQueries.java
View File

@ -761,7 +761,7 @@ public class SessionQueries {
INNER_JOIN + ServerTable.TABLE_NAME + " s on s." + ServerTable.ID + '=' + SessionsTable.TABLE_NAME + '.' + SessionsTable.SERVER_ID +
WHERE + SessionsTable.SESSION_END + ">=?" +
AND + SessionsTable.SESSION_START + "<=?" +
GROUP_BY + "s." + ServerTable.ID;
GROUP_BY + "s." + ServerTable.ID + ",s." + ServerTable.NAME;
return new QueryStatement<>(sql, 100) {
@Override
public void prepare(PreparedStatement statement) throws SQLException {

19
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/objects/TPSQueries.java
View File

@ -482,28 +482,39 @@ public class TPSQueries {
public static Query<Optional<Long>> fetchLatestServerStartTime(ServerUUID serverUUID, long dataGapThreshold) {
String selectPreviousRowNumber = SELECT +
"-1+ROW_NUMBER() over (ORDER BY " + DATE + ") AS previous_rn, " +
SERVER_ID + ',' +
DATE + " AS d1" +
FROM + TABLE_NAME +
WHERE + SERVER_ID + '=' + ServerTable.SELECT_SERVER_ID +
GROUP_BY + SERVER_ID + ',' + DATE +
ORDER_BY + "d1 DESC";
String selectRowNumber = SELECT +
"ROW_NUMBER() over (ORDER BY " + DATE + ") AS rn, " +
SERVER_ID + ',' +
DATE + " AS previous_date" +
FROM + TABLE_NAME +
WHERE + SERVER_ID + '=' + ServerTable.SELECT_SERVER_ID +
GROUP_BY + SERVER_ID + ',' + DATE +
ORDER_BY + "previous_date DESC";
String selectFirstEntryDate = SELECT + "MIN(" + DATE + ") as start_time" +
String selectFirstEntryDate = SELECT +
"MIN(" + DATE + ") as start_time," +
SERVER_ID + " as server_id" +
FROM + TABLE_NAME +
WHERE + SERVER_ID + '=' + ServerTable.SELECT_SERVER_ID;
WHERE + SERVER_ID + '=' + ServerTable.SELECT_SERVER_ID +
GROUP_BY + SERVER_ID;
// Finds the start time since difference between d1 and previous date is a gap,
// so d1 is always first entry after a gap in the data. MAX finds the latest.
// Union ensures if there are no gaps to use the first date recorded.
String selectStartTime = SELECT +
"MAX(d1) AS start_time" +
"MAX(d1) AS start_time," +
"t1." + SERVER_ID + " as server_id" +
FROM + "(" + selectPreviousRowNumber + ") t1" +
INNER_JOIN +
"(" + selectRowNumber + ") t2 ON t1.previous_rn=t2.rn" +
"(" + selectRowNumber + ") t2 ON t1.previous_rn=t2.rn AND t2." + SERVER_ID + "=t1." + SERVER_ID +
WHERE + "d1 - previous_date > ?" +
GROUP_BY + "t1." + SERVER_ID +
UNION + selectFirstEntryDate;
return new QueryStatement<>(selectStartTime) {

2
Plan/common/src/main/java/com/djrapitops/plan/storage/database/queries/objects/WorldTimesQueries.java
View File

@ -135,7 +135,7 @@ public class WorldTimesQueries {
SELECT_WORLD_TIMES_JOIN_WORLD_NAME +
INNER_JOIN + ServerTable.TABLE_NAME + " s on " + WorldTimesTable.TABLE_NAME + '.' + WorldTimesTable.SERVER_ID + "=s." + ServerTable.ID +
WHERE + WorldTimesTable.TABLE_NAME + '.' + WorldTimesTable.USER_ID + "=" + UsersTable.SELECT_USER_ID +
GROUP_BY + WORLD_COLUMN + ',' + WorldTimesTable.TABLE_NAME + '.' + WorldTimesTable.SERVER_ID;
GROUP_BY + WORLD_COLUMN + ",s." + ServerTable.SERVER_UUID;
return new QueryStatement<>(sql, 1000) {
@Override

19
Plan/common/src/test/java/com/djrapitops/plan/storage/database/MySQLTest.java
View File

@ -24,8 +24,10 @@ import com.djrapitops.plan.identification.ServerInfo;
import com.djrapitops.plan.identification.ServerUUID;
import com.djrapitops.plan.settings.config.PlanConfig;
import com.djrapitops.plan.storage.database.queries.ExtensionsDatabaseTest;
import com.djrapitops.plan.storage.database.queries.QueryAllStatement;
import com.djrapitops.plan.storage.database.queries.filter.QueryFilters;
import com.djrapitops.plan.storage.database.transactions.StoreServerInformationTransaction;
import com.djrapitops.plan.storage.database.transactions.Transaction;
import com.djrapitops.plan.storage.database.transactions.commands.RemoveEverythingTransaction;
import com.djrapitops.plan.storage.database.transactions.init.CreateTablesTransaction;
import org.junit.jupiter.api.AfterAll;
@ -42,6 +44,8 @@ import utilities.TestConstants;
import utilities.TestErrorLogger;
import java.nio.file.Path;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -77,6 +81,21 @@ class MySQLTest implements DatabaseTest, DatabaseTestAggregate {
Assumptions.assumeTrue(mysql.isPresent());
database = mysql.get();
database.executeTransaction(new CreateTablesTransaction());
// Enables more strict query mode to prevent errors from it going unnoticed.
database.executeTransaction(new Transaction() {
@Override
protected void performOperations() {
String currentSQLMode = query(new QueryAllStatement<>("SELECT @@GLOBAL.sql_mode as mode") {
@Override
public String processResults(ResultSet set) throws SQLException {
return set.next() ? set.getString("mode") : null;
}
});
if (!currentSQLMode.contains("ONLY_FULL_GROUP_BY")) {
execute("SET GLOBAL sql_mode=(SELECT CONCAT(@@GLOBAL.sql_mode, ',ONLY_FULL_GROUP_BY'))");
}
}
});
}
@BeforeEach

2
Plan/common/src/test/java/com/djrapitops/plan/storage/database/queries/ActivityIndexQueriesTest.java
View File

@ -247,7 +247,7 @@ public interface ActivityIndexQueriesTest extends DatabaseTestPreparer {
assertNotNull(result);
}
@RepeatedTest(25)
@RepeatedTest(5)
default void countRegularPlayers() {
storeSessions(session -> true);
long playtimeThreshold = TimeUnit.MILLISECONDS.toMillis(1L);