[DSIP-89][SqlTask] Avoid using PrepareStatement in SqlTask

[ruanwenjun]

Search before asking

• I had searched in the DSIP and found no similar DSIP.

Motivation

Right now we're using PrepareStatement in SqlTask, and if we find the sql contains ${xx} will using ? to replase ${xx} and set the value in PrepareStatement.

There might exist a lot of bug in this way, some are from DS, some are from third-part driver.
e.g. #17124

So it's better to directly replace the value in sql rather than using Preparement.

Design Detail

No response

Compatibility, Deprecation, and Migration Plan

No response

Test Plan

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[yingh0ng]

Generally, we use PrepareStatement to avoid SQL injection. If we replace the value in sql directly, how do we avoid SQL injection?
If i missing somthing, please tell me.

[ruanwenjun]

Generally, we use PrepareStatement to avoid SQL injection. If we replace the value in sql directly, how do we avoid SQL injection? If i missing somthing, please tell me.

SqlTask don't need to consider sql injection problem, since you can directly construct the injected sql.

[yingh0ng]

Generally, we use PrepareStatement to avoid SQL injection. If we replace the value in sql directly, how do we avoid SQL injection? If i missing somthing, please tell me.

SqlTask don't need to consider sql injection problem, since you can directly construct the injected sql.

Get it!

[qiuyanjun888]

I looked into the current SqlTask implementation and the related issues/PRs (#17124, #17343, #18231). I agree that the current PreparedStatement-based replacement is a bad fit for some SqlTask scenarios, especially when ${} is used in SQL identifiers or DDL statements such as create table test_${dd}. In those cases the placeholder becomes a JDBC parameter marker, which can produce invalid SQL semantics depending on the driver.

Proposed direction

Change SqlTask parameter handling from JDBC bind parameters to SqlTask-local SQL rendering before execution.

1. Keep the change local to SqlTask

The first implementation should avoid changing global ParameterUtils behavior or datasource plugins. It should be limited to the SQL task execution path:

• render ${} / !{} into the SQL string before execution;
• execute the rendered SQL with Statement rather than PreparedStatement;
• preserve existing SQL splitting, pre/main/post execution order, query timeout, max rows, cancel support, result processing, alert attachment, output params and varPool behavior.

This keeps the first PR smaller than the old WIP #17343 and reduces conflict with other task plugins.

2. Define rendering rules explicitly

The main compatibility question is how ${} should behave after removing PreparedStatement binding.

Possible rule set:

• numeric/boolean parameters render as unquoted literals;
• string/date/time/timestamp parameters render as escaped SQL string literals, at least escaping single quotes (' -> '');
• list parameters render as comma-separated literals for IN (...) use cases;
• !{} keeps raw textual replacement semantics for cases where users intentionally need raw SQL fragments;
• identifier/DDL cases such as test_${dd} should work when the rendered value is intended as an identifier fragment.

The difficult part is that ${name} can be used either as a value literal or as part of an identifier. We should agree on this before implementation, because existing SQL like where name=${name} worked with PreparedStatement even without quotes.

3. Suggested implementation shape

• Add a small SqlTask-local renderer with unit tests.
• Adapt SqlBinds or introduce a new internal value object so the name no longer implies JDBC binding.
• Replace prepareStatementAndBind() with a createStatement() helper that still sets query timeout / max rows and stores sessionStatement for cancellation.
• Update query/update execution to call Statement.executeQuery(renderedSql) / Statement.executeUpdate(renderedSql).
• Coordinate with [Fix-18074][Task-Plugin] Fix SQL task parameter passing and unsafe regex replacement #18231 so regex/raw replacement escaping is not solved twice in conflicting ways.

4. Test plan

A first PR should include tests for:

• create table test_${dd} rendering to create table test_20250411;
• string literal escaping;
• list parameter rendering for in (${ids});
• !{} replacement with $ or \ characters;
• schedule-time replacement still working;
• existing result-processing and out-param behavior still working.

Questions for maintainers

1. Should ${} become a value-literal renderer with automatic quoting for strings, or should it be raw textual replacement according to the DSIP wording?
2. Should !{} keep its current raw replacement semantics, or should it be deprecated/merged with ${}?
3. Is the preferred first implementation a small SqlTask-local change, or should it revive the larger execution-engine refactor attempted in [WIP] Avoid using PrepareStatement in SqlTask #17343?
4. Should the first implementation use a generic escaping rule across datasources, or should datasource-specific rendering be introduced from the beginning?
5. How should this work be sequenced with the active parameter replacement PR [Fix-18074][Task-Plugin] Fix SQL task parameter passing and unsafe regex replacement #18231?

If this direction is acceptable, I can prepare a focused PR starting with renderer tests.