This is an automated email from the ASF dual-hosted git repository.
mattcasters 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 3803f8fb6a fix performance regression in combination lookup/update ,
fixes #8922 (#8925)
3803f8fb6a is described below
commit 3803f8fb6a4c010edd69e8fb987a370991e1909b
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Wed Oct 14 23:31:31 2026 +0200
fix performance regression in combination lookup/update , fixes #8922
(#8925)
and an extra performance bump for DimensionLookup
---
.../combinationlookup/CombinationLookup.java | 144 ++++++++++++-------
.../combinationlookup/CombinationLookupData.java | 4 +
.../combinationlookup/CombinationLookupTest.java | 159 +++++++++++++++++++++
.../dimensionlookup/DimensionLookup.java | 30 +++-
.../DimensionLookupTechnicalKeyTest.java | 22 +++
5 files changed, 305 insertions(+), 54 deletions(-)
diff --git
a/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookup.java
b/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookup.java
index eafbf4ee13..331d682c2c 100644
---
a/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookup.java
+++
b/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookup.java
@@ -87,7 +87,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
private void determineTechKeyCreation() {
String keyCreation =
meta.getFields().getReturnFields().getTechKeyCreation();
- DatabaseMeta databaseMeta =
getPipelineMeta().findDatabase(meta.getConnectionName(), variables);
+ DatabaseMeta databaseMeta = data.db.getDatabaseMeta();
if (databaseMeta.supportsAutoinc()
&& CombinationLookupMeta.CREATION_METHOD_AUTOINC.equals(keyCreation)) {
setTechKeyCreation(CREATION_METHOD_AUTOINC);
@@ -99,15 +99,51 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
}
}
- private Long lookupInCache(IRowMeta rowMeta, Object[] row) {
+ /**
+ * A cache key that carries the hash code of the key fields, so a lookup
calculates it once for
+ * the hash field, the cache lookup and the cache store.
+ */
+ static final class CacheKey extends RowMetaAndData {
+ private final int hash;
+
+ CacheKey(IRowMeta rowMeta, Object[] row, int hash) {
+ super(rowMeta, row);
+ this.hash = hash;
+ }
+
+ @Override
+ public int hashCode() {
+ return hash;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ // The key fields decide, the hash is only the hash code of those fields
+ return super.equals(obj);
+ }
+ }
+
+ /**
+ * The cache key of a row. The hash this transform calculates for the hash
field is the same hash
+ * code, so it is reused. A hash read from the stream can be any number, so
the cache does not use
+ * it: entries loaded from the table are keyed by the hash code of the key
fields.
+ */
+ CacheKey cacheKey(Object[] hashRow, Long valHash) throws HopValueException {
+ if (valHash != null && data.hashFieldNr < 0) {
+ return new CacheKey(data.hashRowMeta, hashRow, valHash.intValue());
+ }
+ return new CacheKey(data.hashRowMeta, hashRow,
data.hashRowMeta.hashCode(hashRow));
+ }
+
+ private Long lookupInCache(CacheKey key) {
// Short circuit if cache is disabled.
- if (meta.getCacheSize() == -1) {
+ if (key == null) {
return null;
}
// try to find the row in the cache...
//
- return data.cache.get(new RowMetaAndData(rowMeta, row));
+ return data.cache.get(key);
}
/**
@@ -117,19 +153,18 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
* <p>Key: - natural key fields Value: - Technical key - lookup fields /
extra fields (allows us
* to compare or retrieve) - Date_from - Date_to
*
- * @param rowMeta
- * @param row
+ * @param cacheKey
* @param tk
* @throws HopValueException
*/
- private void addToCache(IRowMeta rowMeta, Object[] row, Long tk) throws
HopValueException {
+ private void addToCache(CacheKey cacheKey, Long tk) throws HopValueException
{
// Short circuit if cache is disabled.
- if (meta.getCacheSize() == -1) {
+ if (cacheKey == null) {
return;
}
// store it in the cache if needed.
- data.cache.put(new RowMetaAndData(rowMeta, row), tk);
+ data.cache.put(cacheKey, tk);
// check if the size is not too big...
// Allow for a buffer overrun of 20% and then remove those 20% in one go.
@@ -196,7 +231,11 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
}
if (isRowLevel()) {
- logRowlevel("Cache store: key=" + rowMeta.getString(row) + " key=" +
tk);
+ logRowlevel(
+ "Cache store: key="
+ + cacheKey.getRowMeta().getString(cacheKey.getData())
+ + " key="
+ + tk);
}
}
@@ -204,54 +243,32 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
return techKeyCreation == CREATION_METHOD_AUTOINC;
}
- private Object[] lookupValues(IRowMeta rowMeta, Object[] row) throws
HopException {
+ Object[] lookupValues(IRowMeta rowMeta, Object[] row) throws HopException {
Long valKey = null;
Long valHash = null;
- Object[] hashRow = null;
+ CacheKey cacheKey = null;
CFields fields = meta.getFields();
List<KeyField> keyFields = fields.getKeyFields();
ReturnFields returnFields = fields.getReturnFields();
- Object[] lookupRow = new Object[data.lookupRowMeta.size()];
- int lookupIndex = 0;
-
if (meta.isUseHash() || meta.getCacheSize() >= 0) {
- hashRow = new Object[data.hashRowMeta.size()];
+ Object[] hashRow = new Object[data.hashRowMeta.size()];
for (int i = 0; i < keyFields.size(); i++) {
hashRow[i] = row[data.keynrs[i]];
}
if (meta.isUseHash()) {
valHash = hashValue(rowMeta, row, hashRow);
- lookupRow[lookupIndex] = valHash;
- lookupIndex++;
}
- }
-
- for (int i = 0; i < keyFields.size(); i++) {
- // Determine the index of this Key Field in the row meta/data
- int rowIndex = data.keynrs[i];
- lookupRow[lookupIndex] = row[rowIndex]; // KEYi = ?
- lookupIndex++;
-
- DatabaseMeta databaseMeta =
- getPipelineMeta().findDatabase(meta.getConnectionName(), variables);
- if (databaseMeta.requiresCastToVariousForIsNull()
- && rowMeta.getValueMeta(rowIndex).getType() ==
IValueMeta.TYPE_STRING) {
- lookupRow[lookupIndex] =
- rowMeta.getValueMeta(rowIndex).isNull(row[rowIndex]) ? null :
"NotNull"; // KEYi IS
- // NULL or
- // ? IS
- // NULL
- } else {
- lookupRow[lookupIndex] = row[data.keynrs[i]]; // KEYi IS NULL or ? IS
NULL
+ if (meta.getCacheSize() >= 0) {
+ cacheKey = cacheKey(hashRow, valHash);
}
- lookupIndex++;
}
// Before doing the actual lookup in the database, see if it's not in the
cache...
- valKey = lookupInCache(data.hashRowMeta, hashRow);
+ valKey = lookupInCache(cacheKey);
if (valKey == null) {
+ Object[] lookupRow = lookupRow(rowMeta, row, valHash);
data.db.setValues(data.lookupRowMeta, lookupRow,
data.prepStatementLookup);
Object[] add = data.db.getLookup(data.prepStatementLookup);
incrementLinesInput();
@@ -293,7 +310,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
}
// Also store it in our Hashtable...
- addToCache(data.hashRowMeta, hashRow, valKey);
+ addToCache(cacheKey, valKey);
} else {
// Entry already exists...
//
@@ -301,7 +318,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
data.db
.getReturnRowMeta()
.getInteger(add, 0); // Sometimes it's not an integer, believe
it or not.
- addToCache(data.hashRowMeta, hashRow, valKey);
+ addToCache(cacheKey, valKey);
}
}
@@ -329,6 +346,35 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
return outputRow;
}
+ /**
+ * The parameters of the lookup statement: the hash (if used), then for
every key KEYi = ? and
+ * KEYi IS NULL AND ? IS NULL. Only needed when the cache misses.
+ */
+ private Object[] lookupRow(IRowMeta rowMeta, Object[] row, Long valHash)
+ throws HopValueException {
+ Object[] lookupRow = new Object[data.lookupRowMeta.size()];
+ int lookupIndex = 0;
+
+ if (meta.isUseHash()) {
+ lookupRow[lookupIndex++] = valHash;
+ }
+
+ for (int i = 0; i < data.keynrs.length; i++) {
+ // Determine the index of this Key Field in the row meta/data
+ int rowIndex = data.keynrs[i];
+ lookupRow[lookupIndex++] = row[rowIndex]; // KEYi = ?
+
+ if (data.nullCheckFlags[i]) {
+ // CAST(? AS VARCHAR(256)) IS NULL
+ lookupRow[lookupIndex++] =
+ rowMeta.getValueMeta(rowIndex).isNull(row[rowIndex]) ? null :
"NotNull";
+ } else {
+ lookupRow[lookupIndex++] = row[rowIndex]; // ? IS NULL
+ }
+ }
+ return lookupRow;
+ }
+
Long hashValue(IRowMeta rowMeta, Object[] row, Object[] hashRow) throws
HopValueException {
if (data.hashFieldNr >= 0) {
return rowMeta.getInteger(row, data.hashFieldNr);
@@ -364,8 +410,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
data.outputRowMeta = getInputRowMeta().clone();
meta.getFields(data.outputRowMeta, getTransformName(), null, null, this,
metadataProvider);
- DatabaseMeta databaseMeta =
- getPipelineMeta().findDatabase(meta.getConnectionName(), variables);
+ DatabaseMeta databaseMeta = data.db.getDatabaseMeta();
data.schemaTable =
databaseMeta.getQuotedSchemaTableCombination(
this, data.realSchemaName, data.realTableName);
@@ -441,10 +486,11 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
}
public void setCombiLookup(IRowMeta inputRowMeta) throws
HopDatabaseException {
- DatabaseMeta databaseMeta =
getPipelineMeta().findDatabase(meta.getConnectionName(), variables);
+ DatabaseMeta databaseMeta = data.db.getDatabaseMeta();
CFields fields = meta.getFields();
List<KeyField> keyFields = fields.getKeyFields();
ReturnFields returnFields = fields.getReturnFields();
+ data.nullCheckFlags = new boolean[keyFields.size()];
String sql = "";
boolean comma;
@@ -489,6 +535,9 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
sql += " IS NULL AND ";
if (databaseMeta.requiresCastToVariousForIsNull()) {
sql += "CAST(? AS VARCHAR(256)) IS NULL";
+ // A string key is bound as a null/not-null flag
+ data.nullCheckFlags[i] =
+ inputRowMeta.getValueMeta(data.keynrs[i]).getType() ==
IValueMeta.TYPE_STRING;
} else {
sql += "? IS NULL";
}
@@ -518,7 +567,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
public Long combiInsert(IRowMeta rowMeta, Object[] row, Long valKey, Long
valCrc)
throws HopDatabaseException {
String debug = "Combination insert";
- DatabaseMeta databaseMeta =
getPipelineMeta().findDatabase(meta.getConnectionName(), variables);
+ DatabaseMeta databaseMeta = data.db.getDatabaseMeta();
CFields fields = meta.getFields();
List<KeyField> keyFields = fields.getKeyFields();
ReturnFields returnFields = fields.getReturnFields();
@@ -789,8 +838,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
throw new HopConfigException(
BaseMessages.getString(PKG,
CONST_COMBINATION_LOOKUP_LOG_UNEXPECTED_ERROR));
}
- DatabaseMeta databaseMeta =
- getPipelineMeta().findDatabase(meta.getConnectionName(), variables);
+ DatabaseMeta databaseMeta = data.db.getDatabaseMeta();
if (databaseMeta == null) {
throw new HopConfigException(
BaseMessages.getString(PKG,
CONST_COMBINATION_LOOKUP_LOG_UNEXPECTED_ERROR));
@@ -853,7 +901,7 @@ public class CombinationLookup extends
BaseTransform<CombinationLookupMeta, Comb
// Assumes the technical key is at position 0 !!
System.arraycopy(cacheRow, 1, hashRow, 0, hashRow.length);
// Potential Cache Overflow is ahndled inside
- addToCache(hashRowMeta, hashRow, (Long) cacheRow[0]);
+ addToCache(cacheKey(hashRow, null), (Long) cacheRow[0]);
incrementLinesInput();
}
}
diff --git
a/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupData.java
b/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupData.java
index 1c3e1a2313..5a44f083f0 100644
---
a/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupData.java
+++
b/plugins/transforms/combinationlookup/src/main/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupData.java
@@ -29,6 +29,10 @@ import org.apache.hop.pipeline.transform.ITransformData;
public class CombinationLookupData extends BaseTransformData implements
ITransformData {
public Database db;
public int[] keynrs; // nrs in row of the keys
+
+ /** Per key: bind a null/not-null flag for "? IS NULL", the database casts
it to VARCHAR */
+ public boolean[] nullCheckFlags;
+
public int hashFieldNr = -1;
public Map<RowMetaAndData, Long> cache;
diff --git
a/plugins/transforms/combinationlookup/src/test/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupTest.java
b/plugins/transforms/combinationlookup/src/test/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupTest.java
new file mode 100644
index 0000000000..002c901cfe
--- /dev/null
+++
b/plugins/transforms/combinationlookup/src/test/java/org/apache/hop/pipeline/transforms/combinationlookup/CombinationLookupTest.java
@@ -0,0 +1,159 @@
+/*
+ * 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.hop.pipeline.transforms.combinationlookup;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.sql.PreparedStatement;
+import java.util.HashMap;
+import org.apache.hop.core.HopEnvironment;
+import org.apache.hop.core.RowMetaAndData;
+import org.apache.hop.core.database.Database;
+import org.apache.hop.core.database.DatabaseMeta;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.row.IRowMeta;
+import org.apache.hop.core.row.RowMeta;
+import org.apache.hop.core.row.value.ValueMetaInteger;
+import org.apache.hop.core.row.value.ValueMetaString;
+import org.apache.hop.pipeline.PipelineMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+class CombinationLookupTest {
+
+ private CombinationLookupMeta meta;
+ private CombinationLookupData data;
+ private PipelineMeta pipelineMeta;
+ private CombinationLookup transform;
+ private Database db;
+ private RowMeta rowMeta;
+
+ @BeforeAll
+ static void setUpEnvironment() throws Exception {
+ HopEnvironment.init();
+ PluginRegistry.init();
+ }
+
+ @BeforeEach
+ void setUp() {
+ meta = new CombinationLookupMeta();
+ meta.setConnectionName("dwh");
+ meta.getFields().getKeyFields().add(new KeyField("country", "country"));
+ meta.getFields().getKeyFields().add(new KeyField("status", "status"));
+ data = new CombinationLookupData();
+ TransformMeta transformMeta = new TransformMeta("lookup", meta);
+ pipelineMeta = spy(new PipelineMeta());
+ pipelineMeta.addTransform(transformMeta);
+ transform = new CombinationLookup(transformMeta, meta, data, 0,
pipelineMeta, null);
+
+ rowMeta = new RowMeta();
+ rowMeta.addValueMeta(new ValueMetaString("country"));
+ rowMeta.addValueMeta(new ValueMetaString("status"));
+ rowMeta.addValueMeta(new ValueMetaInteger("hash"));
+
+ db = mock(Database.class);
+ when(db.getDatabaseMeta()).thenReturn(mock(DatabaseMeta.class));
+ data.db = db;
+ data.prepStatementLookup = mock(PreparedStatement.class);
+ data.keynrs = new int[] {0, 1};
+ data.hashRowMeta = new RowMeta();
+ data.hashRowMeta.addValueMeta(rowMeta.getValueMeta(0));
+ data.hashRowMeta.addValueMeta(rowMeta.getValueMeta(1));
+ data.outputRowMeta = rowMeta.clone();
+ data.outputRowMeta.addValueMeta(new ValueMetaInteger("tk"));
+ data.cache = new HashMap<>();
+ }
+
+ /** The connection was loaded from the metadata for every key of every row
(B418, 200x). */
+ @Test
+ void cacheHitDoesNotLoadTheConnectionOrQueryTheDatabase() throws Exception {
+ data.cache.put(new RowMetaAndData(data.hashRowMeta, "BE", "open"), 7L);
+
+ for (int i = 0; i < 3; i++) {
+ Object[] output = transform.lookupValues(rowMeta, new Object[] {"BE",
"open", 0L});
+ assertEquals(7L, output[3]);
+ }
+
+ verify(pipelineMeta, never()).findDatabase(anyString());
+ verify(pipelineMeta, never()).findDatabase(anyString(), any());
+ verify(pipelineMeta, never()).findDatabase(anyString(), any(),
anyBoolean());
+ verify(db, never()).setValues(any(IRowMeta.class), any(Object[].class),
any());
+ verify(db, never()).getLookup(any(PreparedStatement.class));
+ }
+
+ @Test
+ void cacheMissBindsTheLookupRowAndCachesTheKey() throws Exception {
+ data.lookupRowMeta = new RowMeta();
+ for (int i = 0; i < 4; i++) {
+ data.lookupRowMeta.addValueMeta(new ValueMetaString("lookup" + i));
+ }
+ data.nullCheckFlags = new boolean[] {true, false};
+ RowMeta returnRowMeta = new RowMeta();
+ returnRowMeta.addValueMeta(new ValueMetaInteger("tk"));
+ when(db.getLookup(any(PreparedStatement.class))).thenReturn(new Object[]
{5L});
+ when(db.getReturnRowMeta()).thenReturn(returnRowMeta);
+
+ Object[] row = new Object[] {"BE", "open", 0L};
+ assertEquals(5L, transform.lookupValues(rowMeta, row)[3]);
+ assertEquals(5L, transform.lookupValues(rowMeta, row)[3]);
+
+ ArgumentCaptor<Object[]> lookupRow =
ArgumentCaptor.forClass(Object[].class);
+ verify(db, times(1)).setValues(any(IRowMeta.class), lookupRow.capture(),
any());
+ // KEYi = ? and the IS NULL check: a flag where the database casts it,
else the value
+ assertArrayEquals(new Object[] {"BE", "NotNull", "open", "open"},
lookupRow.getValue());
+ verify(db, times(1)).getLookup(any(PreparedStatement.class));
+ }
+
+ @Test
+ void calculatedHashIsAlsoTheCacheHash() throws Exception {
+ meta.setUseHash(true);
+ data.cache.put(new RowMetaAndData(data.hashRowMeta.clone(), "BE", "open"),
7L);
+ data.hashRowMeta = spy(data.hashRowMeta);
+
+ Object[] output = transform.lookupValues(rowMeta, new Object[] {"BE",
"open", 0L});
+
+ assertEquals(7L, output[3]);
+ verify(data.hashRowMeta, times(1)).hashCode(any(Object[].class));
+ }
+
+ /** A hash read from the stream can be any number, the cache keeps the key
fields' hash code. */
+ @Test
+ void streamHashIsNotTheCacheHash() throws Exception {
+ meta.setUseHash(true);
+ data.hashFieldNr = 2;
+ data.cache.put(new RowMetaAndData(data.hashRowMeta, "BE", "open"), 7L);
+
+ Object[] output = transform.lookupValues(rowMeta, new Object[] {"BE",
"open", 42L});
+
+ assertEquals(7L, output[3]);
+ verify(db, never()).getLookup(any(PreparedStatement.class));
+ }
+}
diff --git
a/plugins/transforms/dimensionlookup/src/main/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookup.java
b/plugins/transforms/dimensionlookup/src/main/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookup.java
index 032338ab4a..e8cd9f8925 100644
---
a/plugins/transforms/dimensionlookup/src/main/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookup.java
+++
b/plugins/transforms/dimensionlookup/src/main/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookup.java
@@ -24,6 +24,7 @@ import static
org.apache.hop.pipeline.transforms.dimensionlookup.DimensionLookup
import static
org.apache.hop.pipeline.transforms.dimensionlookup.DimensionLookupMeta.StartDateAlternative.NULL;
import static
org.apache.hop.pipeline.transforms.dimensionlookup.DimensionLookupMeta.StartDateAlternative.SYSTEM_DATE;
+import java.math.BigDecimal;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
@@ -74,6 +75,9 @@ public class DimensionLookup extends
BaseTransform<DimensionLookupMeta, Dimensio
int[] columnLookupArray = null;
+ /** Describes the lookup date for the conversion to the cache's date-from
type */
+ private final IValueMeta lookupDateMeta = new ValueMetaDate("lookupDate");
+
public DimensionLookup(
TransformMeta transformMeta,
DimensionLookupMeta meta,
@@ -499,7 +503,7 @@ public class DimensionLookup extends
BaseTransform<DimensionLookupMeta, Dimensio
* used to throw, and it should not take a database to find that out again.
*/
static Object asType(IValueMeta targetMeta, Object value) throws
HopValueException {
- if (value == null || targetMeta == null) {
+ if (value == null || targetMeta == null ||
isNativeValueOf(targetMeta.getType(), value)) {
return value;
}
IValueMeta actualMeta = ValueMetaFactory.guessValueMetaInterface(value);
@@ -509,7 +513,22 @@ public class DimensionLookup extends
BaseTransform<DimensionLookupMeta, Dimensio
return targetMeta.convertData(actualMeta, value);
}
- private synchronized Object[] lookupValues(IRowMeta rowMeta, Object[] row)
throws HopException {
+ /**
+ * Whether the value already is what the type stores, decided by what the
value is. It runs for
+ * every row, so it creates no value metadata: that is expensive to
construct.
+ */
+ private static boolean isNativeValueOf(int type, Object value) {
+ return switch (type) {
+ case IValueMeta.TYPE_INTEGER -> value instanceof Long;
+ case IValueMeta.TYPE_BIGNUMBER -> value instanceof BigDecimal;
+ case IValueMeta.TYPE_STRING -> value instanceof String;
+ case IValueMeta.TYPE_NUMBER -> value instanceof Double;
+ case IValueMeta.TYPE_BINARY -> value instanceof byte[];
+ default -> false;
+ };
+ }
+
+ private Object[] lookupValues(IRowMeta rowMeta, Object[] row) throws
HopException {
DLFields f = meta.getFields();
// Determine the lookup date ("now") if we have a field that carries said
@@ -565,9 +584,8 @@ public class DimensionLookup extends
BaseTransform<DimensionLookupMeta, Dimensio
//
IValueMeta preloadFromDateMeta =
data.preloadCache.getRowMeta().getValueMeta(data.preloadFromDateIndex);
- IValueMeta valueDateMeta = new ValueMetaDate("lookupDate");
lookupRow[data.preloadFromDateIndex] =
- preloadFromDateMeta.convertData(valueDateMeta, valueDate);
+ preloadFromDateMeta.convertData(lookupDateMeta, valueDate);
// Look up the row in the pre-load cache...
//
@@ -747,8 +765,8 @@ public class DimensionLookup extends
BaseTransform<DimensionLookupMeta, Dimensio
valueVersion = data.returnRowMeta.getInteger(returnRow, 1);
// Date range: ]-oo,+oo[
- valueDateFrom = meta.getMinDate();
- valueDateTo = meta.getMaxDate();
+ valueDateFrom = data.minDate;
+ valueDateTo = data.maxDate;
// The other values, we compare with
int cmp;
diff --git
a/plugins/transforms/dimensionlookup/src/test/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookupTechnicalKeyTest.java
b/plugins/transforms/dimensionlookup/src/test/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookupTechnicalKeyTest.java
index 21d4f0a511..30c840ac07 100644
---
a/plugins/transforms/dimensionlookup/src/test/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookupTechnicalKeyTest.java
+++
b/plugins/transforms/dimensionlookup/src/test/java/org/apache/hop/pipeline/transforms/dimensionlookup/DimensionLookupTechnicalKeyTest.java
@@ -21,15 +21,21 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
import java.math.BigDecimal;
import org.apache.hop.core.exception.HopValueException;
import org.apache.hop.core.row.IValueMeta;
import org.apache.hop.core.row.value.ValueMetaBigNumber;
import org.apache.hop.core.row.value.ValueMetaBinary;
+import org.apache.hop.core.row.value.ValueMetaFactory;
import org.apache.hop.core.row.value.ValueMetaInteger;
import org.apache.hop.core.row.value.ValueMetaString;
import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
/**
* The technical key travels between two descriptions that need not agree: the
key column decides
@@ -62,6 +68,22 @@ class DimensionLookupTechnicalKeyTest {
assertEquals(42L, handedOn);
}
+ /**
+ * This runs for every output row. Guessing the value's metadata constructs
a value meta, which is
+ * expensive (B436), so a value that already has the declared type must not
need one.
+ */
+ @Test
+ void aValueThatAlreadyMatchesCreatesNoValueMetadata() throws
HopValueException {
+ try (MockedStatic<ValueMetaFactory> factory =
+ mockStatic(ValueMetaFactory.class, CALLS_REAL_METHODS)) {
+ DimensionLookup.asType(new ValueMetaInteger("dimension_id"), 42L);
+ DimensionLookup.asType(new ValueMetaBigNumber("dimension_id", 38, 0),
BigDecimal.ONE);
+ DimensionLookup.asType(new ValueMetaString("dimension_id", 36, 0),
"a-uuid");
+
+ factory.verify(() -> ValueMetaFactory.guessValueMetaInterface(any()),
never());
+ }
+ }
+
@Test
void aValueThatAlreadyMatchesIsPassedThroughUntouched() throws
HopValueException {
Long key = 42L;
