fix(tableinput): don't bind incoming rows when the SQL has no placeholders

docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/tableinput.adoc

@@ -85,6 +85,14 @@ Prefer *named parameters* `{openfield}fieldName{closefield}` so the SQL does not
 
 *Execute for each row* is the usual case: run the query once per incoming row. Disable it only for the legacy assemble-all path, where every incoming row is concatenated into one parameter list. That is how positional SQL such as `WHERE bar IN (?,?,?)` can be filled from three one-field rows.
 
+When the SQL has no bind placeholder at all (no `{openfield}fieldName{closefield}` and no positional `?`), Table Input never binds incoming rows to the statement.
+
+With *Execute for each row* **off**, the incoming hops are still consumed so an upstream transform can complete before the query runs (header or sequencing hops), but their rows are only drained, never concatenated into a parameter list and never bound, and the query runs once without parameters.
+With *Execute for each row* **on**, the query runs once per incoming row (the usual contract), still with no parameter bound.
+Binding values to a statement without placeholders fails on every driver (for example `ORA-17003 Invalid column index` on Oracle), so this is what keeps those pipelines working.
+
+NOTE: This restores the 2.19 behavior for SQL without placeholders and keeps the 2.20 "optional lookup" feature for SQL with placeholders: named and positional parameters are still read from every incoming hop (even with an empty *Insert data from transform*).
+
 If two hops into Table Input have different layouts, Hop still warns about mixing rows. That check is unchanged.
 
 .Named parameter with an informational hop from the previous transform:
@@ -208,7 +216,7 @@ TIP: If you are getting unexpected query results, try clearing the database cach
 
 TIP: A cartesian join transform will combine a different number of fields from multiple table inputs without requiring key join fields.
 
-TIP: When *Execute for each row* is off, Table Input waits until every incoming hop has completed so it can concatenate the parameter rows.
+TIP: When *Execute for each row* is off, Table Input waits until every incoming hop has completed so it can concatenate the parameter rows. When the SQL has no bind placeholder, the incoming rows are only drained so an upstream header or sequencing transform can complete before the query runs.
 
 TIP: For better performance with large datasets you can use indexed columns in `WHERE` clauses and avoid `SELECT *` and only retrieve needed fields.
 
@@ -271,8 +279,8 @@ On the *Fields* tab you can define the output schema instead of asking the datab
 |SQL tab|SQL statement, optional SQL file, `Get SQL select statement`, `Use named parameters`, and `Insert field...` for `{openfield}fieldName{closefield}`.
 |Use named parameters|When enabled, `{openfield}fieldName{closefield}` in SQL is bound to an incoming field. Off for existing Table Input metadata, on for new transforms. Disable if curly braces are used for something else. Checking this option also enables *Execute for each row* when hops exist, and selects the incoming transform when there is only one.
 |Replace variables in script?|Enable to substitute variables (e.g., `+${param}+`) in your SQL before execution.
-|Insert data from transform|Optional. Names the hop that should be treated as informational (parameter rows, not the SQL result). Parameter values are read from **all** incoming hops. Leave empty when you only need the connected hops as parameter sources.
-|Execute for each row?|When incoming hops exist, run the SQL query once for each incoming row. Disable to concatenate all incoming rows into a single parameter list (legacy `WHERE bar IN (?,?,?)` from several one-field rows). Incoming hops with different layouts still produce the mixed-layout warning.
+|Insert data from transform|Optional. Names the hop that should be treated as informational (parameter rows, not the SQL result). Parameter values are read from **all** incoming hops. Leave empty when you only need the connected hops as parameter sources. When the SQL has no bind placeholder, the incoming rows are never bound (only drained when *Execute for each row* is off).
+|Execute for each row?|When incoming hops exist, run the SQL query once for each incoming row. Disable to concatenate all incoming rows into a single parameter list (legacy `WHERE bar IN (?,?,?)` from several one-field rows). Incoming hops with different layouts still produce the mixed-layout warning. When the SQL has no bind placeholder, nothing is ever bound: with this option **off** the incoming rows are only drained and the query runs once without parameters; with it **on** the query runs once per incoming row, still without parameters.
 |Limit size|Maximum number of rows to return from the query. `0` means no limit. See <<limit-size-vs-sql-limit,Limit size vs. SQL LIMIT>> below.
 |Specify output fields|Define the output field list instead of reading column metadata from the database.
 |Validate specified fields|When specifying fields, fail if the query result names or types do not match the list.

plugins/transforms/tableinput/pom.xml

@@ -29,4 +29,29 @@
     <packaging>jar</packaging>
     <name>Hop Plugins Transforms Table Input</name>
 
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>org.apache.hop</groupId>
+                <artifactId>hop-libs-jdbc</artifactId>
+                <version>${project.version}</version>
+                <type>pom</type>
+                <scope>import</scope>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
+    <dependencies>
+        <dependency>
+            <groupId>com.h2database</groupId>
+            <artifactId>h2</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.hop</groupId>
+            <artifactId>hop-databases-h2</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
 </project>

plugins/transforms/tableinput/src/main/java/org/apache/hop/pipeline/transforms/tableinput/TableInput.java

@@ -98,9 +98,40 @@ public boolean processRow() throws HopException {
         if (isDetailed()) {
           logDetailed("Reading all parameter rows from incoming hops");
         }
-        RowMetaAndData assembled = readAllParameterRows();
-        parameters = assembled.getData();
-        parametersMeta = assembled.getRowMeta();
+
+        // Legacy sequencing contract (2.19): when there is no info stream (empty lookup) and no
+        // named parameters, the incoming rows are not parameters at all. They are only consumed
+        // so an upstream "header" transform can complete before the extraction runs. Consuming
+        // them as parameter rows would bind them to a statement without placeholders (ORA-17003 on
+        // Oracle). Drain them without collecting.
+        //
+        if (Utils.isEmpty(meta.getLookup()) && !meta.isUseNamedParameters()) {
+          String resolved;
+          try {
+            resolved = resolveSql();
+          } catch (HopException e) {
+            logError("Could not get SQL: " + e.getMessage());
+            setErrors(1);
+            stopAll();
+            return false;
+          }
+          if (TableInputSql.countPositionalPlaceholders(resolved) == 0) {
+            while (getRow() != null) {
+              // consume the sequencing row(s); nothing is bound
+            }
+            parameters = new Object[] {};
+            parametersMeta = new RowMeta();
+          } else {
+            RowMetaAndData assembled = readAllParameterRows();
+            parameters = assembled.getData();
+            parametersMeta = assembled.getRowMeta();
+          }
+        } else {
+          RowMetaAndData assembled = readAllParameterRows();
+          parameters = assembled.getData();
+          parametersMeta = assembled.getRowMeta();
+        }
+
         if (parameters == null) {
           parameters = new Object[] {};
         }
@@ -205,22 +236,27 @@ private void closePreviousQuery() throws HopDatabaseException {
     }
   }
 
+  private String resolveSql() throws HopException {
+    String sql = meta.getEffectiveSql(variables);
+    if (meta.isVariableReplacementActive()) {
+      sql = resolve(sql);
+    }
+    return sql;
+  }
+
   private boolean doQuery(IRowMeta parametersMeta, Object[] parameters) throws HopException {
     boolean success = true;
 
     // Open the query with the optional parameters received from the source transforms.
     String sql;
     try {
-      sql = meta.getEffectiveSql(variables);
+      sql = resolveSql();
     } catch (HopException e) {
       logError("Could not get SQL: " + e.getMessage());
       setErrors(1);
       stopAll();
       return false;
     }
-    if (meta.isVariableReplacementActive()) {
-      sql = resolve(sql);
-    }
 
     TableInputSql.Bound bound;
     try {

plugins/transforms/tableinput/src/main/java/org/apache/hop/pipeline/transforms/tableinput/TableInputSql.java

@@ -166,12 +166,17 @@ public static Parsed parse(String sql) throws HopException {
   }
 
   /**
-   * Bind incoming row values to the named parameters in {@code parsed}. When the SQL has no named
-   * parameters the original parameter metadata and data are passed through (positional {@code ?}).
+   * Bind incoming row values to the named parameters in {@code parsed}. When the SQL has no
+   * parameters at all (no named, no positional {@code ?}) the bound statement must not receive any
+   * parameter metadata or data: binding values to a statement without placeholders fails on every
+   * driver (ORA-17003 on Oracle, "Parameter index out of range" on H2).
    */
   public static Bound bind(Parsed parsed, IRowMeta parametersMeta, Object[] parameters)
       throws HopException {
     if (!parsed.hasNamedParameters()) {
+      if (parsed.getPositionalParameterCount() == 0) {
+        return new Bound(parsed.getJdbcSql(), null, null);
+      }
       return new Bound(parsed.getJdbcSql(), parametersMeta, parameters);
     }
     if (parametersMeta == null) {
@@ -205,6 +210,80 @@ public static Bound prepare(String sql, IRowMeta parametersMeta, Object[] parame
     return bind(parse(sql), parametersMeta, parameters);
   }
 
+  /**
+   * Count real JDBC positional placeholders ({@code ?}) outside string literals, quoted
+   * identifiers, comments and Hop variables. Used when named parameters are disabled so SQL is
+   * passed through unchanged; values must only be bound when the statement really has placeholders
+   * (otherwise every driver fails: ORA-17003 on Oracle, "Parameter index out of range" on H2).
+   */
+  public static int countPositionalPlaceholders(String sql) {
+    if (sql == null) {
+      return 0;
+    }
+    int count = 0;
+    int i = 0;
+    final int n = sql.length();
+    while (i < n) {
+      char c = sql.charAt(i);
+      char next = (i + 1 < n) ? sql.charAt(i + 1) : 0;
+
+      if (c == '-' && next == '-') {
+        int end = indexOfNewline(sql, i + 2);
+        if (end < 0) {
+          break;
+        }
+        i = end;
+        continue;
+      }
+      if (c == '/' && next == '*') {
+        int end = sql.indexOf("*/", i + 2);
+        if (end < 0) {
+          break;
+        }
+        i = end + 2;
+        continue;
+      }
+      if (c == '\'') {
+        i = skipQuoted(sql, i, '\'');
+        continue;
+      }
+      if (c == '"') {
+        i = skipQuoted(sql, i, '"');
+        continue;
+      }
+      if (c == '$' && next == '{') {
+        int end = sql.indexOf('}', i + 2);
+        if (end < 0) {
+          break;
+        }
+        i = end + 1;
+        continue;
+      }
+      if (c == '?') {
+        count++;
+      }
+      i++;
+    }
+    return count;
+  }
+
+  private static int skipQuoted(String sql, int start, char quote) {
+    int j = start + 1;
+    final int n = sql.length();
+    while (j < n) {
+      char ch = sql.charAt(j);
+      if (ch == quote) {
+        if (j + 1 < n && sql.charAt(j + 1) == quote) {
+          j += 2;
+          continue;
+        }
+        return j + 1;
+      }
+      j++;
+    }
+    return n;
+  }
+
   /**
    * Bind named parameters only when {@code useNamedParameters} is true. Otherwise the SQL is passed
    * through unchanged so existing {@code {braces}} in queries stay literal.
@@ -213,6 +292,9 @@ public static Bound prepare(
       boolean useNamedParameters, String sql, IRowMeta parametersMeta, Object[] parameters)
       throws HopException {
     if (!useNamedParameters) {
+      if (countPositionalPlaceholders(sql) == 0) {
+        return new Bound(sql, null, null);
+      }
       return new Bound(sql, parametersMeta, parameters);
     }
     return prepare(sql, parametersMeta, parameters);

plugins/transforms/tableinput/src/test/java/org/apache/hop/pipeline/transforms/tableinput/TableInputSqlTest.java

@@ -166,11 +166,43 @@ void prepareDisabledLeavesBracesLiteral() throws Exception {
 
     TableInputSql.Bound bound = TableInputSql.prepare(false, sql, incoming, row);
 
+    // No positional placeholder: the SQL is passed through and nothing is bound, otherwise the
+    // driver fails (ORA-17003 / "Parameter index out of range") on a statement without binds.
     assertEquals(sql, bound.getJdbcSql());
+    assertEquals(null, bound.getParameterMeta());
+    assertEquals(null, bound.getParameterData());
+  }
+
+  @Test
+  void prepareDisabledStillBindsWhenPositionalPlaceholdersExist() throws Exception {
+    IRowMeta incoming = new RowMeta();
+    incoming.addValueMeta(new ValueMetaString("key"));
+    Object[] row = new Object[] {"10"};
+
+    TableInputSql.Bound bound =
+        TableInputSql.prepare(false, "SELECT * FROM t WHERE id = ?", incoming, row);
+
+    assertEquals("SELECT * FROM t WHERE id = ?", bound.getJdbcSql());
     assertEquals(incoming, bound.getParameterMeta());
     assertArrayEquals(row, bound.getParameterData());
   }
 
+  @Test
+  void countPositionalPlaceholdersIgnoresLiteralsCommentsAndVariables() {
+    assertEquals(
+        0,
+        TableInputSql.countPositionalPlaceholders(
+            "SELECT '?' AS a, \"q?\" AS b FROM t -- ?\n WHERE x = '${VAR}'"));
+    assertEquals(
+        3,
+        TableInputSql.countPositionalPlaceholders(
+            "SELECT * FROM t WHERE id = ? AND name = ? AND x = ?"));
+    assertEquals(
+        1,
+        TableInputSql.countPositionalPlaceholders(
+            "SELECT * FROM t WHERE id = ? -- comment\n AND /* ? */ name = '?'"));
+  }
+
   @Test
   void parseAllowsSpacesInFieldName() throws Exception {
     TableInputSql.Parsed parsed = TableInputSql.parse("SELECT * FROM t WHERE id = {from date}");