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 70888c1505 Bug #8897 : CONST_IS_NOT_NULL defined as IS NULL produces
invalid SQL (#8906)
70888c1505 is described below
commit 70888c150582d032657c8821113fab93d697854d
Author: Figo Nugroho <[email protected]>
AuthorDate: Wed Oct 14 19:53:16 2026 +0700
Bug #8897 : CONST_IS_NOT_NULL defined as IS NULL produces invalid SQL
(#8906)
* Bug #8897 : CONST_IS_NOT_NULL defined as IS NULL produces invalid SQL
* unit and integration test
---------
Co-authored-by: Hans Van Akelyen <[email protected]>
---
.../org/apache/hop/core/database/Database.java | 2 +-
.../org/apache/hop/core/database/DatabaseTest.java | 77 +++++++
.../database/0007-database-lookup-is-not-null.hpl | 246 +++++++++++++++++++++
.../database/main-0007-input-lookup.hwf | 36 +++
integration-tests/database/scripts/script2.sql | 16 ++
5 files changed, 376 insertions(+), 1 deletion(-)
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 1e65a17d61..626a3197e9 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
@@ -118,7 +118,7 @@ public class Database implements IVariables,
ILoggingObject, AutoCloseable {
private static final String CONST_INOUT = "INOUT";
private static final String CONST_BETWEEN = "BETWEEN";
private static final String CONST_IS_NULL = "IS NULL";
- private static final String CONST_IS_NOT_NULL = "IS NULL";
+ private static final String CONST_IS_NOT_NULL = "IS NOT NULL";
private static final String CONST_BETWEEN_AND = " BETWEEN ? AND ? ";
private static final String CONST_TRUNCATE_NOT_SUPPORTED = "Truncate table
not supported by ";
private static final String CONST_TABLE_CAT = "TABLE_CAT";
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 c9591239d2..fdb08e4ca3 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
@@ -53,6 +53,7 @@ import java.sql.SQLException;
import java.sql.Statement;
import java.sql.Types;
import java.util.List;
+import java.util.regex.Pattern;
import org.apache.hop.core.HopClientEnvironment;
import org.apache.hop.core.Result;
import org.apache.hop.core.exception.HopDatabaseBatchException;
@@ -72,6 +73,7 @@ import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
@ExtendWith(RestoreHopEnvironmentExtension.class)
class DatabaseTest {
@@ -298,6 +300,81 @@ class DatabaseTest {
verify(rsMetaData, times(1)).getColumnCount();
}
+ /**
+ * IS NULL and IS NOT NULL compare against nothing, so the WHERE clause must
not get a "?" for
+ * them. A wrong CONST_IS_NOT_NULL value turned "IS NOT NULL" into "IS NOT
NULL ?" (#8897).
+ */
+ @Test
+ void testSetLookupNullConditionsHaveNoPlaceholder() throws Exception {
+ Database db = databaseForGeneratedSql();
+ db.setLookup(
+ "lookup_table",
+ new String[] {"id", "code", "deleted_at"},
+ new String[] {"=", "IS NOT NULL", "IS NULL"},
+ new String[] {"value"},
+ new String[] {null},
+ null);
+
+ assertNullConditionsHaveNoPlaceholder(capturedSql());
+ }
+
+ @Test
+ void testPrepareUpdateNullConditionsHaveNoPlaceholder() throws Exception {
+ Database db = databaseForGeneratedSql();
+ assertTrue(
+ db.prepareUpdate(
+ "lookup_table",
+ new String[] {"id", "code", "deleted_at"},
+ new String[] {"=", "IS NOT NULL", "IS NULL"},
+ new String[] {"value"}));
+
+ assertNullConditionsHaveNoPlaceholder(capturedSql());
+ }
+
+ @Test
+ void testPrepareDeleteNullConditionsHaveNoPlaceholder() throws Exception {
+ Database db = databaseForGeneratedSql();
+ assertTrue(
+ db.prepareDelete(
+ "lookup_table",
+ new String[] {"id", "code", "deleted_at"},
+ new String[] {"=", "IS NOT NULL", "IS NULL"}));
+
+ assertNullConditionsHaveNoPlaceholder(capturedSql());
+ }
+
+ private Database databaseForGeneratedSql() throws SQLException {
+ when(meta.getQuotedSchemaTableCombination(
+ nullable(IVariables.class), nullable(String.class), anyString()))
+ .thenAnswer(invocation -> invocation.getArgument(2));
+ when(meta.quoteField(anyString())).thenAnswer(invocation ->
invocation.getArgument(0));
+ when(meta.getSqlUpdateStmt(anyString()))
+ .thenAnswer(invocation -> "UPDATE " + invocation.getArgument(0) + "
SET ");
+ when(meta.getSqlDeleteStmt(anyString()))
+ .thenAnswer(invocation -> "DELETE FROM " + invocation.getArgument(0));
+ when(meta.stripCR(anyString())).thenAnswer(invocation ->
invocation.getArgument(0));
+ when(meta.stripCR(any(StringBuilder.class)))
+ .thenAnswer(invocation -> invocation.getArgument(0).toString());
+ when(conn.prepareStatement(anyString())).thenReturn(ps);
+
+ Database db = new Database(log, variables, meta);
+ db.setConnection(conn);
+ return db;
+ }
+
+ private String capturedSql() throws SQLException {
+ ArgumentCaptor<String> sql = ArgumentCaptor.forClass(String.class);
+ verify(conn).prepareStatement(sql.capture());
+ return sql.getValue();
+ }
+
+ private static void assertNullConditionsHaveNoPlaceholder(String sql) {
+ assertTrue(Pattern.compile("id\\s+=\\s+\\?").matcher(sql).find(), sql);
+ assertTrue(sql.contains("code IS NOT NULL"), sql);
+ assertTrue(sql.contains("deleted_at IS NULL"), sql);
+ assertFalse(Pattern.compile("NULL\\s*\\?").matcher(sql).find(), sql);
+ }
+
@Test
void testCreateHopDatabaseBatchExceptionNullUpdatesWhenSqlException() {
assertNull(Database.createHopDatabaseBatchException("", new
SQLException()).getUpdateCounts());
diff --git a/integration-tests/database/0007-database-lookup-is-not-null.hpl
b/integration-tests/database/0007-database-lookup-is-not-null.hpl
new file mode 100644
index 0000000000..282bec8a10
--- /dev/null
+++ b/integration-tests/database/0007-database-lookup-is-not-null.hpl
@@ -0,0 +1,246 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+ <info>
+ <name>0007-database-lookup-is-not-null</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description/>
+ <extended_description/>
+ <pipeline_version/>
+ <pipeline_type>Normal</pipeline_type>
+ <pipeline_status>0</pipeline_status>
+ <parameters>
+ </parameters>
+ <capture_transform_performance>N</capture_transform_performance>
+
<transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+
<transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+ <created_user>-</created_user>
+ <created_date>2026/10/14 12:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2026/10/14 12:00:00.000</modified_date>
+ <key_for_session_key>H4sIAAAAAAAAAAMAAAAAAAAAAAA=</key_for_session_key>
+ <is_key_private>N</is_key_private>
+ </info>
+ <notepads>
+ <notepad>
+ <note>#8897: "code IS NOT NULL" was sent as "code IS NOT NULL ?", which
is invalid SQL.
+Id 1 has a row with a NULL code and one with code A. Fail on multiple results
is on,
+so only a working IS NOT NULL key returns exactly the "code is not null"
row.</note>
+ <xloc>96</xloc>
+ <yloc>256</yloc>
+ <width>500</width>
+ <heigth>58</heigth>
+ <fontname>.AppleSystemUIFont</fontname>
+ <fontsize>13</fontsize>
+ <fontbold>N</fontbold>
+ <fontitalic>N</fontitalic>
+ <fontcolorred>14</fontcolorred>
+ <fontcolorgreen>58</fontcolorgreen>
+ <fontcolorblue>90</fontcolorblue>
+ <backgroundcolorred>201</backgroundcolorred>
+ <backgroundcolorgreen>232</backgroundcolorgreen>
+ <backgroundcolorblue>251</backgroundcolorblue>
+ <bordercolorred>14</bordercolorred>
+ <bordercolorgreen>58</bordercolorgreen>
+ <bordercolorblue>90</bordercolorblue>
+ </notepad>
+ </notepads>
+ <order>
+ <hop>
+ <from>lookup row</from>
+ <to>Database lookup</to>
+ <enabled>Y</enabled>
+ </hop>
+ <hop>
+ <from>Database lookup</from>
+ <to>check value</to>
+ <enabled>Y</enabled>
+ </hop>
+ <hop>
+ <from>check value</from>
+ <to>Abort</to>
+ <enabled>Y</enabled>
+ </hop>
+ <hop>
+ <from>check value</from>
+ <to>success</to>
+ <enabled>Y</enabled>
+ </hop>
+ </order>
+ <transform>
+ <name>lookup row</name>
+ <type>RowGenerator</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <fields>
+ <field>
+ <name>id</name>
+ <type>String</type>
+ <format/>
+ <currency/>
+ <decimal/>
+ <group/>
+ <nullif>1</nullif>
+ <length>-1</length>
+ <precision>-1</precision>
+ <set_empty_string>N</set_empty_string>
+ </field>
+ </fields>
+ <limit>1</limit>
+ <never_ending>N</never_ending>
+ <interval_in_ms>5000</interval_in_ms>
+ <row_time_field>now</row_time_field>
+ <last_time_field>FiveSecondsAgo</last_time_field>
+ <attributes/>
+ <GUI>
+ <xloc>96</xloc>
+ <yloc>80</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Database lookup</name>
+ <type>DBLookup</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <connection>unit-test-db</connection>
+ <cache>N</cache>
+ <cache_load_all>N</cache_load_all>
+ <cache_size>0</cache_size>
+ <lookup>
+ <schema>public</schema>
+ <table>lookup_is_not_null</table>
+ <orderby/>
+ <fail_on_multiple>Y</fail_on_multiple>
+ <eat_row_on_failure>N</eat_row_on_failure>
+ <key>
+ <name>id</name>
+ <field>id</field>
+ <condition>=</condition>
+ <name2/>
+ </key>
+ <key>
+ <name/>
+ <field>code</field>
+ <condition>IS NOT NULL</condition>
+ <name2/>
+ </key>
+ <value>
+ <name>value</name>
+ <rename>value</rename>
+ <default/>
+ <type>String</type>
+ </value>
+ </lookup>
+ <attributes/>
+ <GUI>
+ <xloc>256</xloc>
+ <yloc>80</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>check value</name>
+ <type>FilterRows</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <send_true_to>success</send_true_to>
+ <send_false_to>Abort</send_false_to>
+ <compare>
+ <condition>
+ <negated>N</negated>
+ <leftvalue>value</leftvalue>
+ <function>=</function>
+ <rightvalue/>
+ <value>
+ <name>constant</name>
+ <type>String</type>
+ <text>code is not null</text>
+ <length>-1</length>
+ <precision>-1</precision>
+ <isnull>N</isnull>
+ <mask/>
+ </value>
+ </condition>
+ </compare>
+ <attributes/>
+ <GUI>
+ <xloc>416</xloc>
+ <yloc>80</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>Abort</name>
+ <type>Abort</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <row_threshold>0</row_threshold>
+ <message/>
+ <always_log_rows>Y</always_log_rows>
+ <abort_option>ABORT_WITH_ERROR</abort_option>
+ <attributes/>
+ <GUI>
+ <xloc>416</xloc>
+ <yloc>176</yloc>
+ </GUI>
+ </transform>
+ <transform>
+ <name>success</name>
+ <type>Dummy</type>
+ <description/>
+ <distribute>Y</distribute>
+ <custom_distribution/>
+ <copies>1</copies>
+ <partitioning>
+ <method>none</method>
+ <schema_name/>
+ </partitioning>
+ <attributes/>
+ <GUI>
+ <xloc>576</xloc>
+ <yloc>80</yloc>
+ </GUI>
+ </transform>
+ <transform_error_handling>
+ </transform_error_handling>
+ <attributes/>
+</pipeline>
diff --git a/integration-tests/database/main-0007-input-lookup.hwf
b/integration-tests/database/main-0007-input-lookup.hwf
index 7e38390873..dba455664a 100644
--- a/integration-tests/database/main-0007-input-lookup.hwf
+++ b/integration-tests/database/main-0007-input-lookup.hwf
@@ -162,6 +162,35 @@ limitations under the License.
<yloc>64</yloc>
<attributes_hac/>
</action>
+ <action>
+ <name>0007-database-lookup-is-not-null.hpl</name>
+ <description/>
+ <type>PIPELINE</type>
+ <attributes/>
+ <filename>${PROJECT_HOME}/0007-database-lookup-is-not-null.hpl</filename>
+ <params_from_previous>N</params_from_previous>
+ <exec_per_row>N</exec_per_row>
+ <clear_rows>N</clear_rows>
+ <clear_files>N</clear_files>
+ <set_logfile>N</set_logfile>
+ <logfile/>
+ <logext/>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <loglevel>Basic</loglevel>
+ <set_append_logfile>N</set_append_logfile>
+ <wait_until_finished>Y</wait_until_finished>
+ <follow_abort_remote>N</follow_abort_remote>
+ <create_parent_folder>N</create_parent_folder>
+ <run_configuration>local</run_configuration>
+ <parameters>
+ <pass_all_parameters>Y</pass_all_parameters>
+ </parameters>
+ <parallel>N</parallel>
+ <xloc>1216</xloc>
+ <yloc>64</yloc>
+ <attributes_hac/>
+ </action>
</actions>
<hops>
<hop>
@@ -199,6 +228,13 @@ limitations under the License.
<evaluation>Y</evaluation>
<unconditional>N</unconditional>
</hop>
+ <hop>
+ <from>0007-database-lookup-empty-result.hpl</from>
+ <to>0007-database-lookup-is-not-null.hpl</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
</hops>
<notepads>
</notepads>
diff --git a/integration-tests/database/scripts/script2.sql
b/integration-tests/database/scripts/script2.sql
index 85e77efeb4..83293a69b0 100644
--- a/integration-tests/database/scripts/script2.sql
+++ b/integration-tests/database/scripts/script2.sql
@@ -51,3 +51,19 @@ INSERT INTO foo (id, name, description) VALUES (7, 'Alice',
'Life is what happen
INSERT INTO foo (id, name, description) VALUES (8, 'Bob', 'If life were
predictable it would cease to be life, and be without flavor. -Eleanor
Roosevelt');
INSERT INTO foo (id, name, description) VALUES (9, 'Bob', 'If you look at what
you have in life, youll always have more. -Oprah Winfrey');
INSERT INTO foo (id, name, description) VALUES (10, 'Alice', 'Your time is
limited, dont waste it living someone elses life. -Steve Jobs');
+
+-- #8897: one id with a NULL and a non-NULL code, for the IS NOT NULL lookup
key
+DROP TABLE IF EXISTS public.lookup_is_not_null;
+
+CREATE TABLE public.lookup_is_not_null
+(
+ id varchar,
+ code varchar NULL,
+ value varchar
+);
+
+INSERT INTO public.lookup_is_not_null
+ (id, code, value)
+VALUES ('1', NULL, 'code is null'),
+ ('1', 'A', 'code is not null')
+;
