This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 39bff32a37 Database join performance improvement, fixes #4136 (#7834)
39bff32a37 is described below
commit 39bff32a3738a09790c7a9b4e84b0042739b4bed
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Aug 7 20:33:49 2026 +0200
Database join performance improvement, fixes #4136 (#7834)
---
.../org/apache/hop/core/database/Database.java | 41 +++++++++++-
.../org/apache/hop/core/database/DatabaseTest.java | 75 ++++++++++++++++++++++
.../transforms/databasejoin/DatabaseJoin.java | 1 -
3 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/database/Database.java
b/core/src/main/java/org/apache/hop/core/database/Database.java
index b64ddc6b05..452c01cd7f 100644
--- a/core/src/main/java/org/apache/hop/core/database/Database.java
+++ b/core/src/main/java/org/apache/hop/core/database/Database.java
@@ -147,6 +147,16 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
private IRowMeta rowMeta;
+ /**
+ * The result row metadata derived by {@link #openQuery(PreparedStatement,
IRowMeta, Object[])},
+ * cached together with the prepared statement it was derived from. A
prepared statement always
+ * returns the same result layout, so transforms which re-execute the same
statement once per
+ * incoming row (Database Join) do not have to rebuild it for every single
row.
+ */
+ private PreparedStatement openQueryStatement;
+
+ private IRowMeta openQueryRowMeta;
+
private int written;
private final ILogChannel log;
@@ -647,6 +657,8 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
/** Disconnect from the database and close all open prepared statements. */
public synchronized void disconnect() {
+ openQueryStatement = null;
+ openQueryRowMeta = null;
if (connection == null) {
return; // Nothing to do...
}
@@ -995,6 +1007,7 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
public void closePreparedStatement(PreparedStatement ps) throws
HopDatabaseException {
if (ps != null) {
+ forgetOpenQueryRowMeta(ps);
try {
ps.close();
} catch (SQLException e) {
@@ -1003,6 +1016,18 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
}
}
+ /**
+ * Drop the cached {@link #openQueryRowMeta} when the statement it was
derived from is closed. A
+ * driver is free to hand out the same statement object again for different
SQL afterwards, so the
+ * cached layout must not survive it.
+ */
+ private void forgetOpenQueryRowMeta(PreparedStatement ps) {
+ if (ps == openQueryStatement) {
+ openQueryStatement = null;
+ openQueryRowMeta = null;
+ }
+ }
+
public void closeInsert() throws HopDatabaseException {
if (prepStatementInsert != null) {
try {
@@ -1821,9 +1846,18 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
// to get the length of a String field. So, on MySQL, we ignore the
length
// of Strings in result rows.
//
- log.snap(Metrics.METRIC_DATABASE_GET_ROW_META_START,
databaseMeta.getName());
- rowMeta = getRowInfo(res.getMetaData(), databaseMeta.isMySqlVariant(),
false);
- log.snap(Metrics.METRIC_DATABASE_GET_ROW_META_STOP,
databaseMeta.getName());
+ // A prepared statement always returns the same result layout, so the
row metadata only has to
+ // be derived the first time we see this statement. Rebuilding it on
every execution costs one
+ // plugin class load per column and dominates transforms which execute
the statement once per
+ // incoming row, such as Database Join.
+ //
+ if (ps != openQueryStatement || openQueryRowMeta == null) {
+ log.snap(Metrics.METRIC_DATABASE_GET_ROW_META_START,
databaseMeta.getName());
+ openQueryRowMeta = getRowInfo(res.getMetaData(),
databaseMeta.isMySqlVariant(), false);
+ openQueryStatement = ps;
+ log.snap(Metrics.METRIC_DATABASE_GET_ROW_META_STOP,
databaseMeta.getName());
+ }
+ rowMeta = openQueryRowMeta;
} catch (SQLException ex) {
throw new HopDatabaseException("ERROR executing query", ex);
} catch (Exception e) {
@@ -2466,6 +2500,7 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
selStmt = null;
}
if (pstmt != null) {
+ forgetOpenQueryRowMeta(pstmt);
pstmt.close();
pstmt = null;
}
diff --git a/core/src/test/java/org/apache/hop/core/database/DatabaseTest.java
b/core/src/test/java/org/apache/hop/core/database/DatabaseTest.java
index 77b33eaf3c..6cceafc489 100644
--- a/core/src/test/java/org/apache/hop/core/database/DatabaseTest.java
+++ b/core/src/test/java/org/apache/hop/core/database/DatabaseTest.java
@@ -56,7 +56,9 @@ import org.apache.hop.core.logging.ILoggingObject;
import org.apache.hop.core.logging.LogLevel;
import org.apache.hop.core.row.IRowMeta;
import org.apache.hop.core.row.IValueMeta;
+import org.apache.hop.core.row.RowMeta;
import org.apache.hop.core.row.value.ValueMetaNumber;
+import org.apache.hop.core.row.value.ValueMetaString;
import org.apache.hop.core.util.TestUtil;
import org.apache.hop.core.variables.IVariables;
import org.apache.hop.core.variables.Variables;
@@ -624,4 +626,77 @@ class DatabaseTest {
assertEquals(columnName, iRowMeta.getValueMeta(0).getName());
assertInstanceOf(ValueMetaNumber.class, iRowMeta.getValueMeta(0));
}
+
+ @Test
+ void testOpenQueryDerivesRowMetaOncePerPreparedStatement() throws Exception {
+ Database db = mockDatabaseForOpenQuery();
+
+ // Database Join re-executes the same prepared statement once per incoming
row. Deriving the
+ // result layout costs one plugin class load per column, so it has to
happen only once.
+ //
+ for (int i = 0; i < 5; i++) {
+ db.openQuery(ps, new RowMeta(), new Object[] {});
+
+ assertEquals(1, db.getReturnRowMeta().size());
+ assertEquals(columnName,
db.getReturnRowMeta().getValueMeta(0).getName());
+ assertInstanceOf(ValueMetaNumber.class,
db.getReturnRowMeta().getValueMeta(0));
+ }
+
+ verify(rs, times(1)).getMetaData();
+ }
+
+ @Test
+ void testOpenQueryDerivesRowMetaAgainAfterStatementIsClosed() throws
Exception {
+ Database db = mockDatabaseForOpenQuery();
+
+ db.openQuery(ps, new RowMeta(), new Object[] {});
+ db.closePreparedStatement(ps);
+ db.openQuery(ps, new RowMeta(), new Object[] {});
+
+ // A driver may hand the same statement object back for different SQL once
it is closed, so the
+ // cached layout must not survive it.
+ verify(rs, times(2)).getMetaData();
+ }
+
+ @Test
+ void testOpenQueryDerivesRowMetaPerStatementWhenStatementsAreAlternated()
throws Exception {
+ Database db = mockDatabaseForOpenQuery();
+
+ // A second statement on the same connection, returning a different layout.
+ PreparedStatement otherPs = mock(PreparedStatement.class);
+ ResultSet otherRs = mock(ResultSet.class);
+ ResultSetMetaData otherRsMetaData = mock(ResultSetMetaData.class);
+ when(otherRsMetaData.getColumnCount()).thenReturn(1);
+ when(otherRsMetaData.getColumnName(1)).thenReturn("bonus");
+ when(otherRsMetaData.getColumnLabel(1)).thenReturn("bonus");
+ when(otherRsMetaData.getColumnType(1)).thenReturn(Types.VARCHAR);
+ when(otherRs.getMetaData()).thenReturn(otherRsMetaData);
+ when(otherPs.executeQuery()).thenReturn(otherRs);
+
+ // Alternating statements must never hand out the layout cached for the
other one.
+ //
+ for (int i = 0; i < 3; i++) {
+ db.openQuery(ps, new RowMeta(), new Object[] {});
+ assertEquals(columnName,
db.getReturnRowMeta().getValueMeta(0).getName());
+ assertInstanceOf(ValueMetaNumber.class,
db.getReturnRowMeta().getValueMeta(0));
+
+ db.openQuery(otherPs, new RowMeta(), new Object[] {});
+ assertEquals("bonus", db.getReturnRowMeta().getValueMeta(0).getName());
+ assertInstanceOf(ValueMetaString.class,
db.getReturnRowMeta().getValueMeta(0));
+ }
+ }
+
+ private Database mockDatabaseForOpenQuery() throws SQLException {
+ when(rsMetaData.getColumnCount()).thenReturn(1);
+ when(rsMetaData.getColumnName(1)).thenReturn(columnName);
+ when(rsMetaData.getColumnLabel(1)).thenReturn(columnName);
+ when(rsMetaData.getColumnType(1)).thenReturn(Types.DECIMAL);
+ when(rs.getMetaData()).thenReturn(rsMetaData);
+ when(ps.executeQuery()).thenReturn(rs);
+ when(meta.getIDatabase()).thenReturn(new NoneDatabaseMeta());
+
+ Database db = new Database(log, variables, meta);
+ db.setConnection(conn);
+ return db;
+ }
}
diff --git
a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoin.java
b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoin.java
index 18b3ad35b3..f4cca36ee9 100644
---
a/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoin.java
+++
b/plugins/transforms/databasejoin/src/main/java/org/apache/hop/pipeline/transforms/databasejoin/DatabaseJoin.java
@@ -96,7 +96,6 @@ public class DatabaseJoin extends
BaseTransform<DatabaseJoinMeta, DatabaseJoinDa
data.lookupRowMeta.addValueMeta(rowMeta.getValueMeta(data.keynrs[i]).clone());
}
}
- final ResultSet rs;
try {
// Construct the parameters row...
Object[] lookupRowData = new Object[data.lookupRowMeta.size()];
