mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
@@ -4622,8 +4622,10 @@ } }else if (dialect==Dialect.ORACLE) { try { // oracle has no "create index if not exists"; unquoted identifiers are stored in uppercase if (!isExistsIndex(tableName.toUpperCase(Locale.ROOT),"k_"+tableName.substring("opendj_".length()))) { // oracle has no "create index if not exists"; the lookup spells the table the way this // database stores it - unquoted identifiers go in folded there - and is told so by the // driver rather than by this branch if (!isExistsIndex(tableName,"k_"+tableName.substring("opendj_".length()))) { commitStatement("create index k_"+tableName.substring("opendj_".length())+" on "+tableName+" (k)", true); } }catch (SQLException e) { @@ -5006,8 +5008,15 @@ // class: it asks a data dictionary rather than the data, so a wait here is the metadata lock // of another session - and it is narrowed to the scope every table lookup here is narrowed to return bounded(con, StatementBound.OPERATION, () -> { final DatabaseMetaData metaData=con.getMetaData(); // the table named the way this database stores it, asked of the driver rather than folded // per engine: getIndexInfo() takes a name and matches it against the stored form, and an // unquoted identifier is stored folded. isExistsTable() asks storedIdentifier() the same // question, and an engine wired in later inherits the answer here instead of the upper case // the oracle branch of the caller used to carry for itself (#902) // approximate=true: with false the oracle driver runs ANALYZE on every call try (final ResultSet rs = con.getMetaData().getIndexInfo(scope.catalog, null, tableName, false, true)) { try (final ResultSet rs = metaData.getIndexInfo(scope.catalog, null, storedIdentifier(metaData, tableName), false, true)) { while (rs.next()) { if (indexName.equalsIgnoreCase(rs.getString("INDEX_NAME")) && scope.covers(rs)) { return true; opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
@@ -40,6 +40,7 @@ import java.sql.SQLNonTransientConnectionException; import java.sql.SQLRecoverableException; import java.sql.Statement; import java.util.Locale; import java.util.Properties; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -57,6 +58,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.startsWith; @@ -1064,6 +1066,55 @@ } /** * The table the index guard names to the catalog is spelled the way the database stores it, and which way * that is comes from the driver rather than from the name of the engine. An unquoted identifier is stored * folded - upper case on oracle, lower case on postgresql - and {@link DatabaseMetaData#getIndexInfo} matches * its argument against the stored form and not against the name as it was written, so a guard spelling it * any other way finds no index of a table that carries one and reissues the create behind it: on the two * engines whose {@code create index} has no {@code if not exists} that is the open of the tree failing * (#902). It is the rule {@code isExistsTable()} takes {@code storedIdentifier()} for, and the guard of the * index had it hard-coded in the oracle branch of its caller alone - the one engine of the three that was * known to fold upwards. */ @Test public void testTheIndexGuardNamesTheTableAsTheDatabaseStoresIt() throws Exception { final JDBCStorage storage = storageOverAnEngine(postgresConnection.class, true); final DatabaseMetaData metaData = engineConnection.getMetaData(); // a database of this driver that stores what it is given in upper case: what the guard has to ask about // is then the folded name, whichever branch of openTree() the driver took to get here when(metaData.storesUpperCaseIdentifiers()).thenReturn(true); storage.write(txn -> txn.openTree(TREE, true)); verify(metaData).getIndexInfo(any(), any(), eq(storage.getTableName(TREE).toUpperCase(Locale.ROOT)), anyBoolean(), anyBoolean()); } /** * The other arm of that rule: a driver saying it stores an unquoted identifier as it was written is asked * about the name as it was written. The case above pins the fold alone, and a guard folding upwards * whatever the driver answers would pass it: on oracle the two spellings are one, and on postgresql - where * the stored form is the lower case name the guard was given - the lookup would report no index of a table * that carries one, and the {@code create index if not exists} behind it would reissue in silence, taking * every write that opens a tree out of the conflict replay it is guarded for. Nothing would fail and * nothing would be logged, so what is pinned here is the question being put to the driver rather than the * answer one engine gives. */ @Test public void testTheIndexGuardNamesTheTableAsWrittenWhenTheDatabaseStoresItSo() throws Exception { final JDBCStorage storage = storageOverAnEngine(postgresConnection.class, true); final DatabaseMetaData metaData = engineConnection.getMetaData(); // a database of this driver storing what it is given: storesUpperCaseIdentifiers() and // storesLowerCaseIdentifiers() both answer false, and the name to ask about is the one the caller wrote storage.write(txn -> txn.openTree(TREE, true)); verify(metaData).getIndexInfo(any(), any(), eq(storage.getTableName(TREE)), anyBoolean(), anyBoolean()); } /** * postgresql runs DDL inside the transaction, so a create index the engine rolled back has committed nothing: * {@code write()} rolls the attempt back whole and replays it. Raising the flag in front of the statement - * which is what mysql and oracle need, since they commit before a DDL of their own accord - would turn a opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/PgSqlTestCase.java
@@ -15,6 +15,7 @@ */ package org.opends.server.backends.jdbc; import org.forgerock.opendj.ldap.ByteString; import org.opends.server.backends.pluggable.spi.AccessMode; import org.opends.server.backends.pluggable.spi.TreeName; import org.opends.server.backends.pluggable.spi.WriteOperation; @@ -188,4 +189,115 @@ } } /** The schema of the case below: one no connection of this suite resolves in. */ private static final String OFF_THE_PATH = "opendj_offpath"; /** * A table this connection does not reach unqualified answers for none of this backend's, and neither * does its index (#902). * <p> * A table is named after the hash of its tree name and its index after the table, so both names follow * from the configuration alone: two directories sharing one database, each with a schema and a * {@code search_path} of its own - the routine way to host two of them on one server - hold a table of * the same name and an index of the same name, and nothing about either name tells the two apart. Asked * with no schema at all, as the JDBC contract reads it, the catalog answers for the neighbour's, and both * guards of {@code openTree()} then skip a create this backend needs: the table one leaves every * statement of the backend addressing a relation that is not there, and the index one - the quiet half - * leaves the {@code where k>? order by k} batches of every cursor running unindexed for the life of the * deployment, nothing failing and nothing being logged. * <p> * The neighbour is made by hand rather than by a second storage: what the case needs is a schema the * connections of this one do not resolve in, and the tables of a storage of this suite are made in the * schema they do. */ @Test public void testAnOpenIsAnsweredForByNoTableOfASchemaOffTheSearchPath() throws Exception { final TreeName tree = new TreeName("testOffThePath", "tree"); final JDBCStorage storage = new JDBCStorage(createBackendCfg(getBackendId() + "_offPath"), null); final String tableName = storage.getTableName(tree); final String indexName = "k_" + tableName.substring("opendj_".length()); try { // the schema an unqualified create of this storage lands in, which is where the table and the // index this case is about have to end up final String working; try (final Connection con = DriverManager.getConnection(getJdbcUrl())) { working = con.getSchema(); } try (final Connection con = DriverManager.getConnection(getJdbcUrl()); final Statement st = con.createStatement()) { st.execute("create schema if not exists " + OFF_THE_PATH); // the fixture is the whole of the case: the guards read the whole search_path // (schemaPathOf(), current_schemas(true)) rather than current_schema() alone, so a // neighbour anywhere on it - not only in the schema the connection happens to work in - // would be legitimately found and nothing created, which is not the collision this case // is about try (final ResultSet rs = st.executeQuery("select unnest(current_schemas(true))")) { while (rs.next()) { assertNotEquals(rs.getString(1), OFF_THE_PATH, "the neighbour of this case is on the search_path of the connections of this suite"); } } // the neighbouring directory: the same table and the same index, in a schema this storage // reaches through no unqualified name of its own. Spelled out rather than opened by a // storage, so that the fixture is the collision and nothing else st.execute("create table " + OFF_THE_PATH + "." + tableName + " (h char(128),k bytea,v bytea,primary key(h,k))"); st.execute("create index " + indexName + " on " + OFF_THE_PATH + "." + tableName + " (k)"); } assertFalse(isExistsTableInSchema(working, tableName), "the case did not start with the table of this backend absent from the schema it works in"); storage.open(AccessMode.READ_WRITE); storage.write(new WriteOperation() { @Override public void run(WriteableTransaction txn) throws Exception { txn.openTree(tree, true); // the destructive half of the table guard, and the reason it is loud: found abroad, the // table is created nowhere and this statement addresses a relation that is not there txn.put(tree, ByteString.valueOfUtf8("a key of this backend"), ByteString.valueOfUtf8("a value of this backend")); } }); assertTrue(isExistsTableInSchema(working, tableName), "the open took the table of a schema it does not reach unqualified for its own and created none"); assertTrue(isExistsIndexInSchema(working, indexName), "the open took the index of a schema it does not reach unqualified for its own: the cursor batches of this tree are full scans behind it"); assertEquals(rowCountInSchema(working, tableName), 1, "the write of this backend landed in a table other than the one the open made"); } finally { clearQuietly(storage); try (final Connection con = DriverManager.getConnection(getJdbcUrl()); final Statement st = con.createStatement()) { st.execute("drop schema if exists " + OFF_THE_PATH + " cascade"); } } } /** * Whether that one schema holds the index, which is the index half of the case above and the question * {@code getIndexInfo()} cannot be trusted with here: it is the very lookup under test. */ private boolean isExistsIndexInSchema(String schema, String indexName) throws SQLException { try (final Connection con = DriverManager.getConnection(getJdbcUrl()); final PreparedStatement st = con.prepareStatement( "select 1 from pg_indexes where schemaname=? and lower(indexname)=lower(?)")) { st.setString(1, schema); st.setString(2, indexName); try (final ResultSet rs = st.executeQuery()) { return rs.next(); } } } /** What the table of that one schema holds, which says which of the two tables a write went to. */ private int rowCountInSchema(String schema, String tableName) throws SQLException { try (final Connection con = DriverManager.getConnection(getJdbcUrl()); final Statement st = con.createStatement(); final ResultSet rs = st.executeQuery("select count(*) from " + schema + "." + tableName)) { return rs.next() ? rs.getInt(1) : -1; } } } opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
@@ -102,6 +102,14 @@ /** * Backend test classes sharing one database map the same tree names to the same tables, * so a previous run may leave trees behind — including entries encrypted with a lost cipher key. * <p> * Each one is dropped where it was listed: this lookup is narrowed to no schema, so it reports the * tables of the whole database - pgjdbc adds a schema predicate for a pattern that is not null and * for nothing else - while an unqualified {@code drop table} resolves through the search_path alone. * A table left behind outside it would fail to drop, and the failure of one drop is a * {@link SkipException} over the whole class in {@link #setUp()}: the class would go on skipping run * after run, and the leftover would never be dropped. A driver reporting no schema of its own - mysql * names the database in the catalog instead - keeps the name as it was listed. */ static void dropStaleTrees(Connection con) throws SQLException { final List<String> stale = new ArrayList<>(); @@ -109,7 +117,8 @@ while (rs.next()) { final String name = rs.getString("TABLE_NAME"); if (name.toLowerCase().startsWith("opendj_")) { stale.add(name); final String schema = rs.getString("TABLE_SCHEM"); stale.add(schema == null || schema.isEmpty() ? name : schema + "." + name); } } }
