Fix index creation when using MySQL
This commit fixes a MySQL-related bug introduced on July 13 as part
of commit 3bcb1492aa32cc5034d71f8c473f4a3ff9fa7e3b, which added indexes.
Without this fix, indexes cannot be created for a MySQL-backed events
database, and the Gerrit error_log is spammed with errors.
The problem stems from the fact that MySQL has a slightly different
"CREATE INDEX" syntax than H2. MySQL does not support the
"IF NOT EXISTS" modifier. In order to create an index only if it is
not already present, you have to do some prepared-statement trickery
to generate a statement that either adds the index or does nothing.
In this case, we just select the string "Index exists."
In particular, this change adds a more granular database-detection
system to add in proper support for MySQL. Instead of just explicit
support for Postgres or H2, this now detects Postgres, MySQL, and H2.
Without this fix, anyone using MySQL for the events database will see
the following error in the log over and over and over:
[Store events-1] INFO com.ericsson.gerrit.plugins.eventslog.sql.SQLStore : Connected to database
[Store events-1] WARN com.ericsson.gerrit.plugins.eventslog.sql.SQLStore : Cannot start the database. Events will be stored locally until database connection can be established
com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: 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 'IF NOT EXISTS created_idx ON ChangeEvents (date_created);CREATE INDEX IF NOT EXI' at line 1
Change-Id: Ib8a48d55bbb9c797f58beeab47952e66cd20d62a
diff --git a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java
index 05f663f..9e538e6 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLClient.java
@@ -48,7 +48,7 @@
class SQLClient {
private static final Logger log = LoggerFactory.getLogger(SQLClient.class);
private final Gson gson;
- private final boolean isPostgresql;
+ private final SQLDialect databaseDialect;
private HikariDataSource ds;
@@ -56,7 +56,8 @@
ds = new HikariDataSource(config);
gson = new GsonBuilder().registerTypeAdapter(Supplier.class, new SupplierSerializer()).create();
- isPostgresql = config.getJdbcUrl().contains("postgresql");
+
+ databaseDialect = SQLDialect.fromJdbcUrl(config.getJdbcUrl());
}
/**
@@ -65,8 +66,8 @@
* @throws SQLException If there was a problem with the database
*/
void createDBIfNotCreated() throws SQLException {
- execute(SQLTable.createTableQuery(isPostgresql));
- execute(SQLTable.createIndexes(isPostgresql));
+ execute(SQLTable.createTableQuery(databaseDialect));
+ execute(SQLTable.createIndexes(databaseDialect));
}
/**
diff --git a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLDialect.java b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLDialect.java
new file mode 100644
index 0000000..48c246a
--- /dev/null
+++ b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLDialect.java
@@ -0,0 +1,39 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.ericsson.gerrit.plugins.eventslog.sql;
+
+/** SQLDialect provides enumerations for the various supported dialects of SQL. */
+public enum SQLDialect {
+ H2,
+ MYSQL,
+ POSTGRESQL;
+
+ /**
+ * This attempts to determine the SQL dialect from the JDBC URL.
+ * If the URL does not match one of the supported dialects, then
+ * H2 will be returned by default.
+ *
+ * @param jdbcUrl The JDBC URL.
+ * @return The dialect for the JDBC URL.
+ */
+ public static SQLDialect fromJdbcUrl(String jdbcUrl) {
+ if (jdbcUrl.contains("postgresql")) {
+ return POSTGRESQL;
+ } else if (jdbcUrl.contains("mysql")) {
+ return MYSQL;
+ }
+ return H2;
+ }
+}
diff --git a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLTable.java b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLTable.java
index 0db07a2..6093a38 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLTable.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLTable.java
@@ -23,9 +23,28 @@
static final String DATE_ENTRY = "date_created";
static final String EVENT_ENTRY = "event_info";
+ /** This is the name of the index that tracks the created date. */
private static final String CREATED_INDEX = "created_idx";
+ /** This is the name of the index that tracks the project. */
private static final String PROJECT_INDEX = "project_idx";
+ /**
+ * This is the H2 idempotent index-creation query format. Inputs, in order: index-name,
+ * table-name, index-column
+ */
private static final String H2_INDEX_CREATION_FORMAT = "CREATE INDEX IF NOT EXISTS %s ON %s (%s)";
+ /**
+ * This is the MySQL idempotent index-creation query format. Inputs, in order: table-name,
+ * index-name, table-name, index-name, index-column
+ */
+ private static final String MYSQL_INDEX_CREATION_FORMAT =
+ "SET @x := (SELECT COUNT(*) FROM information_schema.statistics WHERE table_name = '%s' AND index_name = '%s' AND table_schema = DATABASE());\n"
+ + "SET @sql := IF( @x > 0, 'SELECT ''Index exists.''', 'ALTER TABLE %s ADD INDEX %s (%s);');\n"
+ + "PREPARE stmt FROM @sql;\n"
+ + "EXECUTE stmt";
+ /**
+ * This is the Postgres idempotent index-creation query format. Inputs, in order: index-name,
+ * index-name, table-name, index-column
+ */
private static final String POSTGRESQL_INDEX_CREATION_FORMAT =
"DO $$\n"
+ "BEGIN\n"
@@ -42,13 +61,17 @@
private SQLTable() {}
- static String createTableQuery(boolean postgresql) {
+ static String createTableQuery(SQLDialect databaseDialect) {
StringBuilder query = new StringBuilder(140);
query.append(format("CREATE TABLE IF NOT EXISTS %s(", TABLE_NAME));
- if (postgresql) {
- query.append(format("%s SERIAL PRIMARY KEY,", PRIMARY_ENTRY));
- } else {
- query.append(format("%s INT AUTO_INCREMENT PRIMARY KEY,", PRIMARY_ENTRY));
+ switch (databaseDialect) {
+ case POSTGRESQL:
+ query.append(format("%s SERIAL PRIMARY KEY,", PRIMARY_ENTRY));
+ break;
+ case MYSQL:
+ case H2:
+ default:
+ query.append(format("%s INT AUTO_INCREMENT PRIMARY KEY,", PRIMARY_ENTRY));
}
query.append(format("%s VARCHAR(255),", PROJECT_ENTRY));
query.append(format("%s TIMESTAMP DEFAULT NOW(),", DATE_ENTRY));
@@ -56,11 +79,19 @@
return query.toString();
}
- static String createIndexes(boolean postgresql) {
- return postgresql ? getPostgresqlQuery() : getH2Query();
+ static String createIndexes(SQLDialect databaseDialect) {
+ switch (databaseDialect) {
+ case POSTGRESQL:
+ return getPostgresqlIndexQuery();
+ case MYSQL:
+ return getMysqlIndexQuery();
+ case H2:
+ default:
+ return getH2IndexQuery();
+ }
}
- private static String getPostgresqlQuery() {
+ private static String getPostgresqlIndexQuery() {
StringBuilder query = new StringBuilder(540);
query.append(
format(
@@ -80,7 +111,29 @@
return query.toString();
}
- private static String getH2Query() {
+ private static String getMysqlIndexQuery() {
+ StringBuilder query = new StringBuilder();
+ query.append(
+ format(
+ MYSQL_INDEX_CREATION_FORMAT,
+ TABLE_NAME,
+ CREATED_INDEX,
+ TABLE_NAME,
+ CREATED_INDEX,
+ DATE_ENTRY));
+ query.append(";");
+ query.append(
+ format(
+ MYSQL_INDEX_CREATION_FORMAT,
+ TABLE_NAME,
+ PROJECT_INDEX,
+ TABLE_NAME,
+ PROJECT_INDEX,
+ PROJECT_ENTRY));
+ return query.toString();
+ }
+
+ private static String getH2IndexQuery() {
StringBuilder query = new StringBuilder();
query.append(format(H2_INDEX_CREATION_FORMAT, CREATED_INDEX, TABLE_NAME, DATE_ENTRY));
query.append(";");
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 4421212..1c74a20 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -26,6 +26,10 @@
plugin.@PLUGIN@.storeUrl
: Specify the path to the directory in which to keep the database. When not
specified, the default path is jdbc:h2:\<gerrit_site>/data/db.
+ Supported database engines:
+* h2 (default)
+* postgresql
+* mysql
plugin.@PLUGIN@.localStorePath
: Specify the path to the directory in which to keep the back up database.
@@ -42,8 +46,12 @@
plugin.@PLUGIN@.urlOptions
: Options to append to the database url. Each option should be specified in a
separate line using the option=value format. For example:
- urlOptions = loglevel=INFO
- urlOptions = logUnclosedConnections=true
+* `urlOptions = loglevel=INFO`
+* `urlOptions = logUnclosedConnections=true`
+
+When using `mysql`, this option must be specified:
+
+* `urlOptions = allowMultiQueries=true`
plugin.@PLUGIN@.maxTries
: Maximum number of times the plugin should attempt to store the event if a
diff --git a/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLDialectTest.java b/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLDialectTest.java
new file mode 100644
index 0000000..60b7cb1
--- /dev/null
+++ b/src/test/java/com/ericsson/gerrit/plugins/eventslog/sql/SQLDialectTest.java
@@ -0,0 +1,38 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.ericsson.gerrit.plugins.eventslog.sql;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class SQLDialectTest {
+ @Test
+ public void defaultIsH2() throws Exception {
+ assertThat(SQLDialect.fromJdbcUrl("")).isEqualTo(SQLDialect.H2);
+ assertThat(SQLDialect.fromJdbcUrl("jdbc:")).isEqualTo(SQLDialect.H2);
+ assertThat(SQLDialect.fromJdbcUrl("jdbc:whatever://")).isEqualTo(SQLDialect.H2);
+ }
+
+ @Test
+ public void mysqlIsParsed() throws Exception {
+ assertThat(SQLDialect.fromJdbcUrl("jdbc:mysql://")).isEqualTo(SQLDialect.MYSQL);
+ }
+
+ @Test
+ public void postgresqlIsParsed() throws Exception {
+ assertThat(SQLDialect.fromJdbcUrl("jdbc:postgresql://")).isEqualTo(SQLDialect.POSTGRESQL);
+ }
+}
