fskorgen opened a new issue, #8224:
URL: https://github.com/apache/hop/issues/8224
### Apache Hop version?
2.19
### Java version?
21
### Operating system
Windows
### What happened?
`Database.setStatementQueryTimeoutSeconds(int)` sets a JDBC
`Statement.setQueryTimeout` that is
applied by a single private helper:
```java
private void applyStatementQueryTimeout(Statement statement) throws
SQLException {
if (statement != null && statementQueryTimeoutSeconds > 0) {
statement.setQueryTimeout(statementQueryTimeoutSeconds);
}
}
```
It is called from exactly two places, both inside `openQuery`.
`execStatement` — which runs
everything that is *not* a query: `INSERT`, `UPDATE`, `DELETE`, `TRUNCATE`,
DDL — creates its
statements without it:
```java
if (params != null) {
PreparedStatement prepStmt =
connection.prepareStatement(databaseMeta.stripCR(sql));
setValues(params, data, prepStmt);
resultSet = prepStmt.execute();
...
} else {
String sqlStripped = databaseMeta.stripCR(sql);
try (Statement stmt = connection.createStatement()) {
resultSet = stmt.execute(sqlStripped);
...
```
So a caller that sets a timeout gets it honoured for `SELECT` and silently
not for anything else.
### Hop's own SQL editor relies on this
`SqlEditor.runSqlScriptWithMonitor` sets the timeout and then runs a whole
script:
```java
int timeoutSeconds = Math.max(0, Const.toInt(variables.resolve(raw), 0));
try (Database db = new Database(loggingObject, variables, databaseMeta)) {
if (timeoutSeconds > 0) {
db.setStatementQueryTimeoutSeconds(timeoutSeconds);
}
db.connect();
...
runScriptStatementsLoop(db, databaseMeta, sqlScript, message, monitor,
nrStats);
```
`runScriptStatementsLoop` sends every non-query statement to
`executeDdlStatement`, which calls
`db.execStatement(...)`. A user who sets `HOP_QUERY_PREVIEW_TIMEOUT` and
runs a script containing an
`ALTER TABLE` or a large `UPDATE` therefore has no timeout on precisely the
statements most likely to
run long — and `SqlEditor` runs behind a modal progress dialog, so HopGui
waits with it.
`TableInput` and `GetTableNames` also call the setter, but only ever query,
so they are unaffected.
### Is the scope deliberate?
The setter's javadoc says "for statements opened by `openQuery(...)`", so
the narrow scope is at
least documented. Two things argue it is still a defect rather than a
decision:
- The variable is named `HOP_QUERY_PREVIEW_TIMEOUT`, but `SqlEditor` — not a
preview — applies it to
a script it knows contains DDL. The one in-tree caller that mixes
statement kinds gets a guarantee
that silently does not hold.
- The narrow scope is invisible at the call site.
`setStatementQueryTimeoutSeconds` reads like a
connection-wide setting, and nothing fails or warns when a DDL statement
escapes it.
If the scoping *is* intended, the setter would at least be worth renaming
(`setQueryStatementTimeoutSeconds`) so callers cannot mistake it for a
general one.
### Steps to reproduce
1. Open the SQL editor on a connection with `HOP_QUERY_PREVIEW_TIMEOUT` set
to, say, `5`.
2. Run a statement that takes longer than that and is not a `SELECT` — for
example a `WAITFOR DELAY`
on SQL Server, `SELECT pg_sleep(30)` wrapped in a `DO` block on
PostgreSQL, or an `UPDATE` over a
large table.
**Expected:** the driver aborts the statement after 5 seconds, as it does
for a slow `SELECT`.
**Actual:** it runs to completion; the timeout is never applied to the
statement.
### Suggested fix
Apply the existing helper in `execStatement` as well, in both branches:
```java
if (params != null) {
PreparedStatement prepStmt =
connection.prepareStatement(databaseMeta.stripCR(sql));
applyStatementQueryTimeout(prepStmt);
...
} else {
try (Statement stmt = connection.createStatement()) {
applyStatementQueryTimeout(stmt);
...
```
`applyStatementQueryTimeout` already no-ops when no timeout is set, so
callers that never call the
setter are unaffected — the change only reaches connections that asked for a
timeout and were not
getting one.
### Issue Priority
Priority: 3
### Issue Component
Component: Database
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
