You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ad...@apache.org on 2018/04/17 14:54:11 UTC
[ambari] branch trunk updated: AMBARI-23587. Schema upgrade fails
on Oracle (#1017)
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new 7195242 AMBARI-23587. Schema upgrade fails on Oracle (#1017)
7195242 is described below
commit 7195242738c5d55a1632a11a46ee7f126147148a
Author: Doroszlai, Attila <64...@users.noreply.github.com>
AuthorDate: Tue Apr 17 16:54:06 2018 +0200
AMBARI-23587. Schema upgrade fails on Oracle (#1017)
---
.../apache/ambari/server/orm/DBAccessorImpl.java | 71 +++++++++++-----------
.../server/orm/helpers/dbms/GenericDbmsHelper.java | 14 +----
.../ambari/server/upgrade/UpgradeCatalog270.java | 28 ++++-----
.../ambari/server/orm/DBAccessorImplTest.java | 54 ++++++++++++++++
4 files changed, 107 insertions(+), 60 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
index ed3f8f2..34f44f6 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
@@ -349,48 +349,41 @@ public class DBAccessorImpl implements DBAccessor {
@Override
public boolean tableHasForeignKey(String tableName, String fkName) throws SQLException {
- DatabaseMetaData metaData = getDatabaseMetaData();
+ return getCheckedForeignKey(tableName, fkName) != null;
+ }
- ResultSet rs = metaData.getImportedKeys(null, dbSchema, convertObjectName(tableName));
+ public String getCheckedForeignKey(String rawTableName, String rawForeignKeyName) throws SQLException {
+ DatabaseMetaData metaData = getDatabaseMetaData();
- if (rs != null) {
- try {
- while (rs.next()) {
- if (StringUtils.equalsIgnoreCase(fkName, rs.getString("FK_NAME"))) {
- return true;
- }
+ String tableName = convertObjectName(rawTableName);
+ String foreignKeyName = convertObjectName(rawForeignKeyName);
+ try (ResultSet rs = metaData.getImportedKeys(null, dbSchema, tableName)) {
+ while (rs.next()) {
+ String foundName = rs.getString("FK_NAME");
+ if (StringUtils.equals(foreignKeyName, foundName)) {
+ return foundName;
}
- } finally {
- rs.close();
}
}
- LOG.warn("FK {} not found for table {}", convertObjectName(fkName), convertObjectName(tableName));
-
- return false;
- }
-
- public String getCheckedForeignKey(String tableName, String fkName) throws SQLException {
- DatabaseMetaData metaData = getDatabaseMetaData();
-
- ResultSet rs = metaData.getImportedKeys(null, dbSchema, convertObjectName(tableName));
-
- if (rs != null) {
- try {
- while (rs.next()) {
- if (StringUtils.equalsIgnoreCase(fkName, rs.getString("FK_NAME"))) {
- return rs.getString("FK_NAME");
+ DatabaseType databaseType = configuration.getDatabaseType();
+ if (databaseType == DatabaseType.ORACLE) {
+ try (PreparedStatement ps = getConnection().prepareStatement("SELECT constraint_name FROM user_constraints WHERE table_name = ? AND constraint_type = 'R' AND constraint_name = ?")) {
+ ps.setString(1, tableName);
+ ps.setString(2, foreignKeyName);
+ try (ResultSet rs = ps.executeQuery()) {
+ if (rs.next()) {
+ return foreignKeyName;
}
}
- } finally {
- rs.close();
}
}
- LOG.warn("FK {} not found for table {}", convertObjectName(fkName), convertObjectName(tableName));
+ LOG.warn("FK {} not found for table {}", foreignKeyName, tableName);
return null;
}
+
@Override
public boolean tableHasForeignKey(String tableName, String refTableName,
String columnName, String refColumnName) throws SQLException {
@@ -1390,14 +1383,22 @@ public class DBAccessorImpl implements DBAccessor {
* the value to escape
* @return the escaped value
*/
- protected String escapeParameter(Object value) {
- // Only String and number supported.
- // Taken from:
- // org.eclipse.persistence.internal.databaseaccess.appendParameterInternal
- Object dbValue = databasePlatform.convertToDatabaseType(value);
+ private String escapeParameter(Object value) {
+ return escapeParameter(value, databasePlatform);
+ }
+
+ public static String escapeParameter(Object value, DatabasePlatform databasePlatform) {
+ if (value == null) {
+ return null;
+ }
+
+ if (value instanceof Enum<?>) {
+ value = ((Enum) value).name();
+ }
+
String valueString = value.toString();
- if (dbValue instanceof String) {
- valueString = "'" + value + "'";
+ if (value instanceof String || databasePlatform.convertToDatabaseType(value) instanceof String) {
+ valueString = "'" + valueString + "'";
}
return valueString;
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
index 24665c8..4871918 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
@@ -24,6 +24,7 @@ import java.io.Writer;
import java.util.List;
import org.apache.ambari.server.orm.DBAccessor;
+import org.apache.ambari.server.orm.DBAccessorImpl;
import org.apache.commons.lang.StringUtils;
import org.eclipse.persistence.internal.databaseaccess.FieldTypeDefinition;
import org.eclipse.persistence.internal.databaseaccess.Platform;
@@ -454,16 +455,7 @@ public class GenericDbmsHelper implements DbmsHelper {
* the value to escape
* @return the escaped value
*/
- protected String escapeParameter(Object value) {
- // Only String and number supported.
- // Taken from:
- // org.eclipse.persistence.internal.databaseaccess.appendParameterInternal
- Object dbValue = databasePlatform.convertToDatabaseType(value);
- String valueString = value.toString();
- if (dbValue instanceof String || dbValue instanceof Enum) {
- valueString = "'" + value + "'";
- }
-
- return valueString;
+ private String escapeParameter(Object value) {
+ return DBAccessorImpl.escapeParameter(value, databasePlatform);
}
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog270.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog270.java
index 0f80cc9..945618a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog270.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog270.java
@@ -543,19 +543,19 @@ public class UpgradeCatalog270 extends AbstractUpgradeCatalog {
dbAccessor.executeUpdate(
"insert into " + temporaryTable +
"(" + USER_AUTHENTICATION_USER_AUTHENTICATION_ID_COLUMN + ", " + USER_AUTHENTICATION_USER_ID_COLUMN + ", " + USER_AUTHENTICATION_AUTHENTICATION_TYPE_COLUMN + ", " + USER_AUTHENTICATION_AUTHENTICATION_KEY_COLUMN + ", " + USER_AUTHENTICATION_CREATE_TIME_COLUMN + ", " + USER_AUTHENTICATION_UPDATE_TIME_COLUMN + ")" +
- " select" +
+ " select distinct" +
" u." + USERS_USER_ID_COLUMN + "," +
" t.min_user_id," +
" u." + USERS_USER_TYPE_COLUMN + "," +
" u." + USERS_USER_PASSWORD_COLUMN + "," +
" u." + USERS_CREATE_TIME_COLUMN + "," +
" u." + USERS_CREATE_TIME_COLUMN +
- " from " + USERS_TABLE + " as u inner join" +
+ " from " + USERS_TABLE + " u inner join" +
" (select" +
" lower(" + USERS_USER_NAME_COLUMN + ") as " + USERS_USER_NAME_COLUMN + "," +
" min(" + USERS_USER_ID_COLUMN + ") as min_user_id" +
" from " + USERS_TABLE +
- " group by lower(" + USERS_USER_NAME_COLUMN + ")) as t" +
+ " group by lower(" + USERS_USER_NAME_COLUMN + ")) t" +
" on (lower(u." + USERS_USER_NAME_COLUMN + ") = lower(t." + USERS_USER_NAME_COLUMN + "))"
);
@@ -571,7 +571,7 @@ public class UpgradeCatalog270 extends AbstractUpgradeCatalog {
dbAccessor.executeUpdate(
"insert into " + USER_AUTHENTICATION_TABLE +
"(" + USER_AUTHENTICATION_USER_AUTHENTICATION_ID_COLUMN + ", " + USER_AUTHENTICATION_USER_ID_COLUMN + ", " + USER_AUTHENTICATION_AUTHENTICATION_TYPE_COLUMN + ", " + USER_AUTHENTICATION_AUTHENTICATION_KEY_COLUMN + ", " + USER_AUTHENTICATION_CREATE_TIME_COLUMN + ", " + USER_AUTHENTICATION_UPDATE_TIME_COLUMN + ")" +
- " select distinct " +
+ " select " +
USER_AUTHENTICATION_USER_AUTHENTICATION_ID_COLUMN + ", " + USER_AUTHENTICATION_USER_ID_COLUMN + ", " + USER_AUTHENTICATION_AUTHENTICATION_TYPE_COLUMN + ", " + USER_AUTHENTICATION_AUTHENTICATION_KEY_COLUMN + ", " + USER_AUTHENTICATION_CREATE_TIME_COLUMN + ", " + USER_AUTHENTICATION_UPDATE_TIME_COLUMN +
" from " + temporaryTable
);
@@ -699,7 +699,7 @@ public class UpgradeCatalog270 extends AbstractUpgradeCatalog {
" m." + MEMBERS_MEMBER_ID_COLUMN + "," +
" u.min_user_id," +
" m." + MEMBERS_GROUP_ID_COLUMN +
- " from " + MEMBERS_TABLE + " as m inner join" +
+ " from " + MEMBERS_TABLE + " m inner join" +
" (" +
" select" +
" iu." + USERS_USER_NAME_COLUMN + "," +
@@ -712,8 +712,8 @@ public class UpgradeCatalog270 extends AbstractUpgradeCatalog {
" min(" + USERS_USER_ID_COLUMN + ") as min_user_id" +
" from " + USERS_TABLE +
" group by lower(" + USERS_USER_NAME_COLUMN + ")" +
- " ) as t on (lower(t." + USERS_USER_NAME_COLUMN + ") = lower(iu." + USERS_USER_NAME_COLUMN + "))" +
- " ) as u on (m." + MEMBERS_USER_ID_COLUMN + " = u." + USERS_USER_ID_COLUMN + ")");
+ " ) t on (lower(t." + USERS_USER_NAME_COLUMN + ") = lower(iu." + USERS_USER_NAME_COLUMN + "))" +
+ " ) u on (m." + MEMBERS_USER_ID_COLUMN + " = u." + USERS_USER_ID_COLUMN + ")");
// Truncate existing membership records
dbAccessor.truncateTable(MEMBERS_TABLE);
@@ -780,7 +780,7 @@ public class UpgradeCatalog270 extends AbstractUpgradeCatalog {
" ap." + ADMINPRIVILEGE_PERMISSION_ID_COLUMN + "," +
" ap." + ADMINPRIVILEGE_RESOURCE_ID_COLUMN + "," +
" ap." + ADMINPRIVILEGE_PRINCIPAL_ID_COLUMN +
- " from " + ADMINPRIVILEGE_TABLE + " as ap" +
+ " from " + ADMINPRIVILEGE_TABLE + " ap" +
" where ap." + ADMINPRIVILEGE_PRINCIPAL_ID_COLUMN + " not in" +
" (" +
" select " + USERS_PRINCIPAL_ID_COLUMN +
@@ -792,29 +792,29 @@ public class UpgradeCatalog270 extends AbstractUpgradeCatalog {
" ap." + ADMINPRIVILEGE_PERMISSION_ID_COLUMN + "," +
" ap." + ADMINPRIVILEGE_RESOURCE_ID_COLUMN + "," +
" t.new_principal_id" +
- " from " + ADMINPRIVILEGE_TABLE + " as ap inner join" +
+ " from " + ADMINPRIVILEGE_TABLE + " ap inner join" +
" (" +
" select" +
" u." + USERS_USER_ID_COLUMN + "," +
" u." + USERS_USER_NAME_COLUMN + "," +
" u." + USERS_PRINCIPAL_ID_COLUMN + " as new_principal_id," +
" t1." + USERS_PRINCIPAL_ID_COLUMN + " as orig_principal_id" +
- " from " + USERS_TABLE + " as u inner join" +
+ " from " + USERS_TABLE + " u inner join" +
" (" +
" select" +
" u1." + USERS_USER_NAME_COLUMN + "," +
" u1." + USERS_PRINCIPAL_ID_COLUMN + "," +
" t2.min_user_id" +
- " from " + USERS_TABLE + " as u1 inner join" +
+ " from " + USERS_TABLE + " u1 inner join" +
" (" +
" select" +
" lower(" + USERS_USER_NAME_COLUMN + ") as " + USERS_USER_NAME_COLUMN + "," +
" min(" + USERS_USER_ID_COLUMN + ") as min_user_id" +
" from " + USERS_TABLE +
" group by lower(" + USERS_USER_NAME_COLUMN + ")" +
- " ) as t2 on (lower(u1." + USERS_USER_NAME_COLUMN + ") = lower(t2." + USERS_USER_NAME_COLUMN + "))" +
- " ) as t1 on (u." + USERS_USER_ID_COLUMN + " = t1.min_user_id)" +
- " ) as t on (ap." + ADMINPRIVILEGE_PRINCIPAL_ID_COLUMN + " = t.orig_principal_id);");
+ " ) t2 on (lower(u1." + USERS_USER_NAME_COLUMN + ") = lower(t2." + USERS_USER_NAME_COLUMN + "))" +
+ " ) t1 on (u." + USERS_USER_ID_COLUMN + " = t1.min_user_id)" +
+ " ) t on (ap." + ADMINPRIVILEGE_PRINCIPAL_ID_COLUMN + " = t.orig_principal_id)");
// Truncate existing adminprivilege records
dbAccessor.truncateTable(ADMINPRIVILEGE_TABLE);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
index 15c83bb..99d463b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
@@ -18,6 +18,9 @@
package org.apache.ambari.server.orm;
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.reset;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -44,6 +47,8 @@ import java.util.concurrent.atomic.AtomicInteger;
import org.apache.ambari.server.H2DatabaseCleaner;
import org.apache.ambari.server.orm.DBAccessor.DBColumnInfo;
+import org.apache.ambari.server.state.State;
+import org.eclipse.persistence.platform.database.DatabasePlatform;
import org.eclipse.persistence.sessions.DatabaseSession;
import org.junit.After;
import org.junit.Before;
@@ -368,6 +373,28 @@ public class DBAccessorImplTest {
}
@Test
+ public void getCheckedForeignKeyReferencingUniqueKey() throws Exception {
+ String tableName = getFreeTableName();
+ createMyTable(tableName);
+
+ DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class);
+ Statement statement = dbAccessor.getConnection().createStatement();
+ statement.execute(String.format("ALTER TABLE %s ADD CONSTRAINT UC_name UNIQUE (%s)", tableName, "name"));
+
+ List<DBColumnInfo> columns = new ArrayList<>();
+ columns.add(new DBColumnInfo("fid", Long.class, null, null, false));
+ columns.add(new DBColumnInfo("fname", String.class, null, null, false));
+
+ String foreignTableName = getFreeTableName();
+ dbAccessor.createTable(foreignTableName, columns);
+
+ statement = dbAccessor.getConnection().createStatement();
+ statement.execute(String.format("ALTER TABLE %s ADD CONSTRAINT FK_name FOREIGN KEY (%s) REFERENCES %s (%s)", foreignTableName, "fname", tableName, "name"));
+
+ Assert.assertEquals("FK_NAME", dbAccessor.getCheckedForeignKey(foreignTableName, "fk_name"));
+ }
+
+ @Test
public void testTableExists() throws Exception {
DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class);
@@ -855,4 +882,31 @@ public class DBAccessorImplTest {
}
}
+ @Test
+ public void escapesEnumValue() {
+ DatabasePlatform platform = createNiceMock(DatabasePlatform.class);
+ Object value = State.UNKNOWN;
+ expect(platform.convertToDatabaseType(value)).andReturn(value).anyTimes();
+ reset(platform);
+ assertEquals("'" + value + "'", DBAccessorImpl.escapeParameter(value, platform));
+ }
+
+ @Test
+ public void escapesString() {
+ DatabasePlatform platform = createNiceMock(DatabasePlatform.class);
+ Object value = "hello, world";
+ expect(platform.convertToDatabaseType(value)).andReturn(value).anyTimes();
+ reset(platform);
+ assertEquals("'" + value + "'", DBAccessorImpl.escapeParameter(value, platform));
+ }
+
+ @Test
+ public void doesNotEscapeNumbers() {
+ DatabasePlatform platform = createNiceMock(DatabasePlatform.class);
+ Object value = 123;
+ expect(platform.convertToDatabaseType(value)).andReturn(value).anyTimes();
+ reset(platform);
+ assertEquals("123", DBAccessorImpl.escapeParameter(value, platform));
+ }
+
}
--
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.