From 1e2e3c91eba5a0ed99b28ce2cc30a3651a534803 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Tue, 22 Sep 2026 09:07:31 +0000
Subject: [PATCH] [#902] Name the table to the index guard of the JDBC backend the way the database stores it (#1001)
---
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java | 11 ++
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java | 51 ++++++++++++
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/PgSqlTestCase.java | 112 ++++++++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java | 15 +++
4 files changed, 185 insertions(+), 4 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
index 4f56aa4..d2f2c61 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
+++ b/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;
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
index 0a20a37..bb0eeb0 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java
+++ b/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
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/PgSqlTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/PgSqlTestCase.java
index 6ff6719..e1c751c 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/PgSqlTestCase.java
+++ b/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;
+ }
+ }
+
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
index 75001d3..da5f705 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
+++ b/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);
}
}
}
--
Gitblit v1.10.0