You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@oozie.apache.org by an...@apache.org on 2018/08/29 14:24:20 UTC

oozie git commit: OOZIE-2684 Bad database schema error for WF_ACTIONS table (abhishekbafna, kmarton via andras.piros)

Repository: oozie
Updated Branches:
  refs/heads/master e08d696e4 -> eeb4529df


OOZIE-2684 Bad database schema error for WF_ACTIONS table (abhishekbafna, kmarton via andras.piros)


Project: http://git-wip-us.apache.org/repos/asf/oozie/repo
Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/eeb4529d
Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/eeb4529d
Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/eeb4529d

Branch: refs/heads/master
Commit: eeb4529df5b3138596e6e02b9b81ead24494a0f0
Parents: e08d696
Author: Andras Piros <an...@cloudera.com>
Authored: Wed Aug 29 16:22:42 2018 +0200
Committer: Andras Piros <an...@cloudera.com>
Committed: Wed Aug 29 16:22:42 2018 +0200

----------------------------------------------------------------------
 .../oozie/command/SchemaCheckXCommand.java      |  9 ++-
 .../apache/oozie/util/db/CompositeIndex.java    | 65 ++++++++++++++++++++
 .../oozie/util/db/TestCompositeIndex.java       | 53 ++++++++++++++++
 release-log.txt                                 |  1 +
 .../java/org/apache/oozie/tools/OozieDBCLI.java | 21 ++-----
 .../org/apache/oozie/tools/TestOozieDBCLI.java  |  5 +-
 6 files changed, 130 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/oozie/blob/eeb4529d/core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java b/core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java
index 521408e..0e69eb2 100644
--- a/core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java
+++ b/core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java
@@ -36,6 +36,7 @@ import org.apache.oozie.sla.SLARegistrationBean;
 import org.apache.oozie.sla.SLASummaryBean;
 import org.apache.oozie.util.Pair;
 import org.apache.oozie.util.XLog;
+import org.apache.oozie.util.db.CompositeIndex;
 import org.apache.openjpa.persistence.jdbc.Index;
 
 import javax.persistence.Column;
@@ -251,7 +252,9 @@ public class SchemaCheckXCommand extends XCommand<Void> {
         ResultSet rs = metaData.getIndexInfo(catalog, null, table, false, true);
         while (rs.next()) {
             String colName = rs.getString("COLUMN_NAME");
-            if (colName != null) {
+            String indexName = rs.getString("INDEX_NAME");
+            final boolean isExtraIndexedColumn = !CompositeIndex.find(indexName) && colName != null;
+            if (isExtraIndexedColumn) {
                 foundIndexedColumns.add(colName);
             }
         }
@@ -432,10 +435,6 @@ public class SchemaCheckXCommand extends XCommand<Void> {
                 columnTypes.put(name, type);
                 indexedColumns.add(name);
             }
-            // For some reason, MySQL doesn't end up having this index...
-            if (dbType.equals("mysql") && clazz.equals(WorkflowActionBean.class)) {
-                indexedColumns.remove("wf_id");
-            }
         }
 
         private static Integer getSQLType(Class<?> clazz, boolean isLob, String dbType) {

http://git-wip-us.apache.org/repos/asf/oozie/blob/eeb4529d/core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java b/core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
new file mode 100644
index 0000000..88b9ca3
--- /dev/null
+++ b/core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.oozie.util.db;
+
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.Lists;
+
+import java.util.Arrays;
+import java.util.List;
+
+public enum CompositeIndex {
+    I_WF_JOBS_STATUS_CREATED_TIME ("WF_JOBS", "status", "created_time"),
+    I_COORD_ACTIONS_JOB_ID_STATUS ("COORD_ACTIONS", "job_id", "status"),
+    I_COORD_JOBS_STATUS_CREATED_TIME ("COORD_JOBS", "status", "created_time"),
+    I_COORD_JOBS_STATUS_LAST_MODIFIED_TIME ("COORD_JOBS", "status", "last_modified_time"),
+    I_COORD_JOBS_PENDING_DONE_MATERIALIZATION_LAST_MODIFIED_TIME
+            ("COORD_JOBS", "pending", "done_materialization", "last_modified_time"),
+    I_COORD_JOBS_PENDING_LAST_MODIFIED_TIME ("COORD_JOBS", "pending", "last_modified_time"),
+    I_BUNLDE_JOBS_STATUS_CREATED_TIME ("BUNDLE_JOBS", "status", "created_time"),
+    I_BUNLDE_JOBS_STATUS_LAST_MODIFIED_TIME ("BUNDLE_JOBS", "status", "last_modified_time"),
+    I_BUNLDE_ACTIONS_PENDING_LAST_MODIFIED_TIME ("BUNDLE_ACTIONS", "pending", "last_modified_time");
+
+    private final String createStatement;
+
+    CompositeIndex(String tableName, String ... columnNames) {
+        final String columns = Joiner.on(", ").join(columnNames);
+        this.createStatement = String.format("CREATE INDEX %s ON %s (%s)",
+                name(), tableName.toUpperCase(), columns);
+    }
+
+    public static List<String> getIndexStatements() {
+        Function<CompositeIndex, String> compositeIndexToString = new Function<CompositeIndex, String>() {
+            public String apply(CompositeIndex i) { return i != null ? i.createStatement : null; }
+        };
+
+        List<CompositeIndex> indexList = Arrays.asList(values());
+        return Lists.transform(indexList, compositeIndexToString);
+    }
+
+    public static boolean find(String indexName) {
+        try {
+            valueOf(indexName.toUpperCase());
+        } catch (IllegalArgumentException | NullPointerException ex) {
+            return false;
+        }
+        return true;
+    }
+}

http://git-wip-us.apache.org/repos/asf/oozie/blob/eeb4529d/core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java b/core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java
new file mode 100644
index 0000000..4a5c6e4
--- /dev/null
+++ b/core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.oozie.util.db;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestCompositeIndex {
+
+    @Test
+    public void testisCompositIndexLowerCase() {
+        Assert.assertTrue("This should be a valid composite index",
+                CompositeIndex.find("i_coord_actions_job_id_status"));
+    }
+
+    @Test
+    public void testisCompositIndexUpperCase() {
+        Assert.assertTrue("This should be a valid composite index",
+                CompositeIndex.find("I_COORD_ACTIONS_JOB_ID_STATUS"));
+    }
+
+    @Test
+    public void testisCompositIndexNull() {
+        Assert.assertFalse("This is not a valid composite index",
+                CompositeIndex.find(null));
+    }
+
+    @Test
+    public void testisCompositIndexEmptyString() {
+        Assert.assertFalse("This is not a valid composite index", CompositeIndex.find(""));
+    }
+
+    @Test
+    public void testIsCompositIndexNegative() {
+        Assert.assertFalse("This is not a valid composite index", CompositeIndex.find("NotAnIndex"));
+    }
+}

http://git-wip-us.apache.org/repos/asf/oozie/blob/eeb4529d/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index 3c569d0..ad517e3 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 5.1.0 release (trunk - unreleased)
 
+OOZIE-2684 Bad database schema error for WF_ACTIONS table (abhishekbafna, kmarton via andras.piros)
 OOZIE-3332 [examples] Spark examples should feature yarn client and cluster modes (daniel.becker via andras.piros)
 OOZIE-3318 [build] Fix Javadoc check in the pre-commit (kmarton via andras.piros)
 OOZIE-3317 [build] Fix false positive precommit reports (kmarton via andras.piros)

http://git-wip-us.apache.org/repos/asf/oozie/blob/eeb4529d/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java
----------------------------------------------------------------------
diff --git a/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java b/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java
index eeaaa60..b421aa6 100644
--- a/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java
+++ b/tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java
@@ -18,6 +18,7 @@
 
 package org.apache.oozie.tools;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.commons.cli.Options;
@@ -29,6 +30,7 @@ import org.apache.oozie.cli.CLIParser;
 import org.apache.oozie.service.ConfigurationService;
 import org.apache.oozie.service.JPAService;
 import org.apache.oozie.service.Services;
+import org.apache.oozie.util.db.CompositeIndex;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -747,23 +749,8 @@ public class OozieDBCLI {
         }
     }
 
-    static String[] getIndexStatementsFor50() {
-        return new String[]{"CREATE INDEX I_WF_JOBS_STATUS_CREATED_TIME ON WF_JOBS (status, created_time)",
-
-                "CREATE INDEX I_COORD_ACTIONS_JOB_ID_STATUS ON COORD_ACTIONS (job_id, status)",
-
-                "CREATE INDEX I_COORD_JOBS_STATUS_CREATED_TIME ON COORD_JOBS (status, created_time)",
-                "CREATE INDEX I_COORD_JOBS_STATUS_LAST_MODIFIED_TIME ON COORD_JOBS (status, last_modified_time)",
-                "CREATE INDEX I_COORD_JOBS_PENDING_DONE_MATERIALIZATION_LAST_MODIFIED_TIME ON COORD_JOBS " +
-                        "(pending, done_materialization, last_modified_time)",
-                "CREATE INDEX I_COORD_JOBS_PENDING_LAST_MODIFIED_TIME ON COORD_JOBS (pending, last_modified_time)",
-
-                "CREATE INDEX I_BUNLDE_JOBS_STATUS_CREATED_TIME ON BUNDLE_JOBS (status, created_time)",
-                "CREATE INDEX I_BUNLDE_JOBS_STATUS_LAST_MODIFIED_TIME ON BUNDLE_JOBS (status, last_modified_time)",
-
-                "CREATE INDEX I_BUNLDE_ACTIONS_PENDING_LAST_MODIFIED_TIME ON BUNDLE_ACTIONS (pending, last_modified_time)"};
-    }
 
+    @SuppressFBWarnings(value = {"SQL_INJECTION_JDBC"}, justification = "Final values are used")
     private void ddlTweaksFor50(final String sqlFile, final boolean run) throws Exception {
         System.out.println("Creating composite indexes");
         try (final Connection conn = createConnection();
@@ -771,7 +758,7 @@ public class OozieDBCLI {
              final Statement stmt = conn.createStatement())
         {
             writer.println();
-            final String[] createCoveringIndexStatements = getIndexStatementsFor50();
+            final List<String> createCoveringIndexStatements = CompositeIndex.getIndexStatements();
 
             for (final String query : createCoveringIndexStatements) {
                 writer.println(query + ";");

http://git-wip-us.apache.org/repos/asf/oozie/blob/eeb4529d/tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java
----------------------------------------------------------------------
diff --git a/tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java b/tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java
index cf3427f..3489f20 100644
--- a/tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java
+++ b/tools/src/test/java/org/apache/oozie/tools/TestOozieDBCLI.java
@@ -23,6 +23,7 @@ import org.apache.oozie.action.hadoop.security.LauncherSecurityManager;
 import org.apache.oozie.service.Services;
 import org.apache.hadoop.fs.FileUtil;
 import org.apache.oozie.test.XTestCase;
+import org.apache.oozie.util.db.CompositeIndex;
 import org.junit.Assert;
 
 import java.io.ByteArrayOutputStream;
@@ -177,7 +178,7 @@ public class TestOozieDBCLI extends XTestCase {
         final Charset charset = Charset.defaultCharset();
         final List<String> stringList = Files.readAllLines(upgrade.toPath(), charset);
         final List<String> actualIndexStatements = Arrays.asList(stringList.toArray(new String[]{}));
-        final String[] expectedIndexStatements = OozieDBCLI.getIndexStatementsFor50();
+        final List<String> expectedIndexStatements = CompositeIndex.getIndexStatements();
 
         for (final String indexStmt : expectedIndexStatements) {
             Assert.assertTrue(actualIndexStatements.contains(indexStmt + ";"));
@@ -186,7 +187,7 @@ public class TestOozieDBCLI extends XTestCase {
 
     private void verifyIndexesCreated() throws SQLException {
         final List<String> indexes = getIndexes();
-        for (final String indexStmt : OozieDBCLI.getIndexStatementsFor50()) {
+        for (final String indexStmt : CompositeIndex.getIndexStatements()) {
             final  String index = indexStmt.split(" ")[2];
             Assert.assertTrue(indexes.contains(index));
         }