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 fb235a21a6 fix field rename edge case and flaky tests, fixes #6995
(#6997)
fb235a21a6 is described below
commit fb235a21a6cc7fa48a83350f926858b6943c86ac
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Tue Apr 14 13:26:46 2026 +0200
fix field rename edge case and flaky tests, fixes #6995 (#6997)
---
.../main/java/org/apache/hop/core/row/RowMeta.java | 27 +++++-
.../hop/core/row/RowMetaAddRemoveValueTest.java | 21 +++++
.../org/apache/hop/pipeline/PipelineMetaTest.java | 2 +
.../copy/DefaultTransformMetaCopyFactoryTest.java | 8 +-
.../hop/pipeline/transforms/sql/ExecSql.java | 9 ++
.../hop/pipeline/transforms/sql/ExecSqlDialog.java | 3 +
.../sql/messages/messages_en_US.properties | 1 +
.../hop/pipeline/transforms/xml/xmloutput/one.txt | 92 ++++++++++-----------
.../hop/pipeline/transforms/xml/xmloutput/two.txt | 96 +++++++++++-----------
9 files changed, 162 insertions(+), 97 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/row/RowMeta.java
b/core/src/main/java/org/apache/hop/core/row/RowMeta.java
index 4b3deeb5d3..5ea4ffdc5d 100644
--- a/core/src/main/java/org/apache/hop/core/row/RowMeta.java
+++ b/core/src/main/java/org/apache/hop/core/row/RowMeta.java
@@ -230,6 +230,29 @@ public class RowMeta implements IRowMeta {
return (meta != null) && searchValueMeta(meta.getName()) != null;
}
+ /**
+ * Resolves an existing column index for duplicate detection. Must be called
with the write lock
+ * held. After {@link #removeValueMeta(int)} the name cache is cleared; this
method still finds
+ * matches by scanning {@link #valueMetaList} (same idea as {@link
#indexOfValue(String)}).
+ */
+ private Integer indexOfExistingValueMetaIgnoreCase(String name) {
+ if (Utils.isEmpty(name)) {
+ return null;
+ }
+ Integer index = cache.findAndCompare(name, valueMetaList);
+ if (index != null) {
+ return index;
+ }
+ for (int i = 0; i < valueMetaList.size(); i++) {
+ if (name.equalsIgnoreCase(valueMetaList.get(i).getName())) {
+ cache.storeMapping(name, i);
+ needRealClone = null;
+ return i;
+ }
+ }
+ return null;
+ }
+
/**
* Add a metadata value. If a value with the same name already exists, it
gets renamed.
*
@@ -241,7 +264,7 @@ public class RowMeta implements IRowMeta {
lock.writeLock().lock();
try {
IValueMeta newMeta;
- Integer existsIdx = cache.findAndCompare(meta.getName(),
valueMetaList);
+ Integer existsIdx = indexOfExistingValueMetaIgnoreCase(meta.getName());
if (existsIdx == null) {
newMeta = meta;
} else {
@@ -270,7 +293,7 @@ public class RowMeta implements IRowMeta {
lock.writeLock().lock();
try {
IValueMeta newMeta;
- Integer existsIdx = cache.findAndCompare(meta.getName(),
valueMetaList);
+ Integer existsIdx = indexOfExistingValueMetaIgnoreCase(meta.getName());
if (existsIdx == null) {
newMeta = meta;
} else {
diff --git
a/core/src/test/java/org/apache/hop/core/row/RowMetaAddRemoveValueTest.java
b/core/src/test/java/org/apache/hop/core/row/RowMetaAddRemoveValueTest.java
index 1c686f437a..4e39c49205 100644
--- a/core/src/test/java/org/apache/hop/core/row/RowMetaAddRemoveValueTest.java
+++ b/core/src/test/java/org/apache/hop/core/row/RowMetaAddRemoveValueTest.java
@@ -173,6 +173,27 @@ class RowMetaAddRemoveValueTest {
assertEquals(0, rowMeta.size());
}
+ /**
+ * After removeValueMeta the name cache is cleared; adding a field that
collides with an existing
+ * name must still rename (e.g. second Database Lookup after Select Values
dropped {@code *_1}).
+ */
+ @Test
+ void duplicateRenameAfterRemovingRenamedField() throws Exception {
+ IRowMeta rowMeta = new RowMeta();
+ rowMeta.addValueMeta(ValueMetaFactory.createValueMeta("brand",
IValueMeta.TYPE_INTEGER));
+ rowMeta.addValueMeta(ValueMetaFactory.createValueMeta("brand",
IValueMeta.TYPE_STRING));
+ assertEquals("brand", rowMeta.getValueMeta(0).getName());
+ assertEquals("brand_1", rowMeta.getValueMeta(1).getName());
+
+ rowMeta.removeValueMeta("brand_1");
+ assertEquals(1, rowMeta.size());
+
+ rowMeta.addValueMeta(ValueMetaFactory.createValueMeta("brand",
IValueMeta.TYPE_STRING));
+ assertEquals(2, rowMeta.size());
+ assertEquals("brand", rowMeta.getValueMeta(0).getName());
+ assertEquals("brand_1", rowMeta.getValueMeta(1).getName());
+ }
+
@Test
void testAddRemoveValueCaseInsensitive() throws Exception {
IRowMeta rowMeta = new RowMeta();
diff --git a/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaTest.java
b/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaTest.java
index c1f924c383..d97c1d6d60 100644
--- a/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaTest.java
+++ b/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaTest.java
@@ -101,6 +101,8 @@ class PipelineMetaTest {
PluginRegistry registry = PluginRegistry.getInstance();
registry.registerPluginClass(
FakeMeta.class.getName(), TransformPluginType.class, Transform.class);
+ registry.registerPluginClass(
+ DummyMeta.class.getName(), TransformPluginType.class, Transform.class);
}
@Test
diff --git
a/engine/src/test/java/org/apache/hop/pipeline/transform/copy/DefaultTransformMetaCopyFactoryTest.java
b/engine/src/test/java/org/apache/hop/pipeline/transform/copy/DefaultTransformMetaCopyFactoryTest.java
index 62d7cfbc1c..54fa97b14b 100644
---
a/engine/src/test/java/org/apache/hop/pipeline/transform/copy/DefaultTransformMetaCopyFactoryTest.java
+++
b/engine/src/test/java/org/apache/hop/pipeline/transform/copy/DefaultTransformMetaCopyFactoryTest.java
@@ -24,6 +24,9 @@ import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.apache.hop.core.annotations.Transform;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.plugins.TransformPluginType;
import org.apache.hop.pipeline.transform.TransformMeta;
import org.apache.hop.pipeline.transforms.dummy.DummyMeta;
import org.junit.jupiter.api.BeforeEach;
@@ -40,7 +43,10 @@ class DefaultTransformMetaCopyFactoryTest {
private DummyMeta dummyMeta;
@BeforeEach
- void setUp() {
+ void setUp() throws Exception {
+ PluginRegistry.getInstance()
+ .registerPluginClass(DummyMeta.class.getName(),
TransformPluginType.class, Transform.class);
+
factory = DefaultTransformMetaCopyFactory.getInstance();
// Create a test transform metadata
diff --git
a/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSql.java
b/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSql.java
index 10719be404..bf63f2900c 100644
---
a/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSql.java
+++
b/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSql.java
@@ -274,6 +274,15 @@ public class ExecSql extends BaseTransform<ExecSqlMeta,
ExecSqlData> {
return false;
}
DatabaseMeta databaseMeta =
getPipelineMeta().findDatabase(meta.getConnection(), variables);
+ if (databaseMeta == null) {
+ logError(
+ BaseMessages.getString(
+ PKG,
+ "ExecSql.Init.ConnectionNotFound",
+ variables.resolve(meta.getConnection()),
+ getTransformName()));
+ return false;
+ }
data.db = new Database(this, this, databaseMeta);
// Connect to the database
diff --git
a/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSqlDialog.java
b/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSqlDialog.java
index 0a96593ed1..8e35d63e40 100644
---
a/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSqlDialog.java
+++
b/plugins/transforms/sql/src/main/java/org/apache/hop/pipeline/transforms/sql/ExecSqlDialog.java
@@ -524,6 +524,9 @@ public class ExecSqlDialog extends BaseTransformDialog {
}
DatabaseMeta databaseMeta =
pipelineMeta.findDatabase(input.getConnection(), variables);
+ if (databaseMeta == null) {
+ return List.of();
+ }
return Arrays.stream(databaseMeta.getReservedWords()).toList();
}
diff --git
a/plugins/transforms/sql/src/main/resources/org/apache/hop/pipeline/transforms/sql/messages/messages_en_US.properties
b/plugins/transforms/sql/src/main/resources/org/apache/hop/pipeline/transforms/sql/messages/messages_en_US.properties
index b4aa4c7d42..d35622f66a 100644
---
a/plugins/transforms/sql/src/main/resources/org/apache/hop/pipeline/transforms/sql/messages/messages_en_US.properties
+++
b/plugins/transforms/sql/src/main/resources/org/apache/hop/pipeline/transforms/sql/messages/messages_en_US.properties
@@ -18,6 +18,7 @@
ExecSql.Description=Execute an SQL script, optionally parameterized using
input rows
ExecSql.Exception.CouldNotFindField=Couldn''t find field ''{0}'' in row\!
ExecSql.Init.ConnectionMissing=Database connection is missing for transform
[{0}]\!
+ExecSql.Init.ConnectionNotFound=No database connection named ''{0}'' could be
found for transform [{1}]\!
ExecSql.Log.ConnectedToDB=Connected to database...
ExecSql.Log.ErrorFindingField=Error finding field\:
ExecSql.Log.ErrorInTransform=Error while running this transform\!
diff --git
a/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/one.txt
b/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/one.txt
index b92e03afce..786f4d8c8e 100644
---
a/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/one.txt
+++
b/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/one.txt
@@ -1,47 +1,47 @@
- <encoding>UTF-8</encoding>
- <name_space/>
- <xml_main_element>Rows</xml_main_element>
- <xml_repeat_element>Row</xml_repeat_element>
- <file>
- <name>xmlOutputFile</name>
- <extention>apache_hop.xml</extention>
- <servlet_output>N</servlet_output>
- <do_not_open_newfile_init>N</do_not_open_newfile_init>
- <split>Y</split>
- <add_date>Y</add_date>
- <add_time>Y</add_time>
- <SpecifyFormat>N</SpecifyFormat>
- <omit_null_values>Y</omit_null_values>
- <date_time_format/>
- <add_to_result_filenames>N</add_to_result_filenames>
- <zipped>N</zipped>
- <splitevery>0</splitevery>
- </file>
- <fields>
- <field>
- <content_type>Element</content_type>
- <name>fieldOne</name>
- <element/>
- <type>Number</type>
- <format/>
- <currency/>
- <decimal/>
- <group/>
- <nullif/>
- <length>-1</length>
- <precision>-1</precision>
- </field>
- <field>
- <content_type>Attribute</content_type>
- <name>fieldTwo</name>
- <element/>
- <type>String</type>
- <format/>
- <currency/>
- <decimal/>
- <group/>
- <nullif/>
- <length>-1</length>
- <precision>-1</precision>
- </field>
+ <encoding>UTF-8</encoding>
+ <name_space/>
+ <xml_main_element>Rows</xml_main_element>
+ <xml_repeat_element>Row</xml_repeat_element>
+ <file>
+ <name>xmlOutputFile</name>
+ <extention>apache_hop.xml</extention>
+ <servlet_output>N</servlet_output>
+ <do_not_open_newfile_init>N</do_not_open_newfile_init>
+ <split>Y</split>
+ <add_date>Y</add_date>
+ <add_time>Y</add_time>
+ <SpecifyFormat>N</SpecifyFormat>
+ <omit_null_values>Y</omit_null_values>
+ <date_time_format/>
+ <add_to_result_filenames>N</add_to_result_filenames>
+ <zipped>N</zipped>
+ <splitevery>0</splitevery>
+ </file>
+ <fields>
+ <field>
+ <content_type>Element</content_type>
+ <name>fieldOne</name>
+ <element/>
+ <type>Number</type>
+ <format/>
+ <currency/>
+ <decimal/>
+ <group/>
+ <nullif/>
+ <length>-1</length>
+ <precision>-1</precision>
+ </field>
+ <field>
+ <content_type>Attribute</content_type>
+ <name>fieldTwo</name>
+ <element/>
+ <type>String</type>
+ <format/>
+ <currency/>
+ <decimal/>
+ <group/>
+ <nullif/>
+ <length>-1</length>
+ <precision>-1</precision>
+ </field>
</fields>
\ No newline at end of file
diff --git
a/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/two.txt
b/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/two.txt
index df23a3708e..8301f6e4b8 100644
---
a/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/two.txt
+++
b/plugins/transforms/xml/src/test/resources/org/apache/hop/pipeline/transforms/xml/xmloutput/two.txt
@@ -1,48 +1,48 @@
-
- <encoding>UTF-8</encoding>
- <name_space/>
- <xml_main_element>Rows</xml_main_element>
- <xml_repeat_element>Row</xml_repeat_element>
- <file>
- <name>xmlOutputFile</name>
- <extention>apache_pentaho.xml</extention>
- <servlet_output>N</servlet_output>
- <do_not_open_newfile_init>N</do_not_open_newfile_init>
- <split>Y</split>
- <add_date>Y</add_date>
- <add_time>Y</add_time>
- <SpecifyFormat>N</SpecifyFormat>
- <omit_null_values>Y</omit_null_values>
- <date_time_format/>
- <add_to_result_filenames>N</add_to_result_filenames>
- <zipped>N</zipped>
- <splitevery>0</splitevery>
- </file>
- <fields>
- <field>
- <content_type>Element</content_type>
- <name>fieldOne</name>
- <element/>
- <type>Number</type>
- <format/>
- <currency/>
- <decimal/>
- <group/>
- <nullif/>
- <length>-1</length>
- <precision>-1</precision>
- </field>
- <field>
- <content_type>Attribute</content_type>
- <name>fieldTwo</name>
- <element/>
- <type>String</type>
- <format/>
- <currency/>
- <decimal/>
- <group/>
- <nullif/>
- <length>-1</length>
- <precision>-1</precision>
- </field>
- </fields>
+
+ <encoding>UTF-8</encoding>
+ <name_space/>
+ <xml_main_element>Rows</xml_main_element>
+ <xml_repeat_element>Row</xml_repeat_element>
+ <file>
+ <name>xmlOutputFile</name>
+ <extention>apache_pentaho.xml</extention>
+ <servlet_output>N</servlet_output>
+ <do_not_open_newfile_init>N</do_not_open_newfile_init>
+ <split>Y</split>
+ <add_date>Y</add_date>
+ <add_time>Y</add_time>
+ <SpecifyFormat>N</SpecifyFormat>
+ <omit_null_values>Y</omit_null_values>
+ <date_time_format/>
+ <add_to_result_filenames>N</add_to_result_filenames>
+ <zipped>N</zipped>
+ <splitevery>0</splitevery>
+ </file>
+ <fields>
+ <field>
+ <content_type>Element</content_type>
+ <name>fieldOne</name>
+ <element/>
+ <type>Number</type>
+ <format/>
+ <currency/>
+ <decimal/>
+ <group/>
+ <nullif/>
+ <length>-1</length>
+ <precision>-1</precision>
+ </field>
+ <field>
+ <content_type>Attribute</content_type>
+ <name>fieldTwo</name>
+ <element/>
+ <type>String</type>
+ <format/>
+ <currency/>
+ <decimal/>
+ <group/>
+ <nullif/>
+ <length>-1</length>
+ <precision>-1</precision>
+ </field>
+ </fields>
