Skip to content

[Fix-18074][Task-Plugin] Fix SQL task parameter passing and unsafe regex replacement#18231

Open
leocook wants to merge 2 commits intoapache:devfrom
leocook:fix-18074-sql-parameter-type-passing
Open

[Fix-18074][Task-Plugin] Fix SQL task parameter passing and unsafe regex replacement#18231
leocook wants to merge 2 commits intoapache:devfrom
leocook:fix-18074-sql-parameter-type-passing

Conversation

@leocook
Copy link
Copy Markdown
Contributor

@leocook leocook commented May 8, 2026

Was this PR generated or assisted by AI?

YES, Opus 4.7

Purpose of the pull request

close #18074

Brief change log

When an upstream SQL task outputs parameter p1 and the downstream task references it via a different parameter name p2 (with value ${p1}), the workflow fails with:

java.lang.IllegalArgumentException: No group with name {p1}
    at java.util.regex.Matcher.appendReplacement(Matcher.java:849)
    at org.apache.dolphinscheduler.plugin.task.sql.SqlTask.replaceOriginalValue(SqlTask.java:496)

Two root causes:

  1. VarPool not injected: CuringParamsServiceImpl.paramParsingPreparation() step 6 only overrides parameters with the same name. Since the downstream task only has p2 and not p1, the upstream VarPool value (p1=111) gets dropped, causing ${p1} to remain unresolved.

  2. Unsafe regex replacement: SqlTask.replaceOriginalValue() uses Matcher.replaceFirst(paramValue) directly, which treats $ as a regex group reference. When the unresolved placeholder ${p1} contains $, it causes the regex engine to crash.

Changes:

  • CuringParamsServiceImpl: Inject VarPool parameters that don't exist in prepareParamsMap for downstream placeholder resolution
  • SqlTask: Use Matcher.quoteReplacement() to safely handle $ in parameter values, and add null safety check

Verify this pull request

This pull request is covered by new unit tests:

  • CuringParamsServiceImplTest.testParamParsingPreparation_varPoolInjectionWhenParamNameNotInLocalParams — verifies upstream VarPool parameter p1 is injected and downstream p2=${p1} is resolved to 111
  • SqlTaskTest.testReplaceOriginalValue_withSpecialCharacters — verifies $, \ characters in values are safely replaced
  • SqlTaskTest.testReplaceOriginalValue_paramNotFound_keepsOriginalText — verifies original text is preserved when parameter is not found
  • SqlTaskTest.testReplaceOriginalValue_paramValueNull_keepsOriginalText — verifies original text is preserved when parameter value is null

Screenshots

Before fix:
image
image

After fix:
image
image

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

Copy link
Copy Markdown
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Varpool only be announced as a in parameter then it can be used, we shouldn't directly put varpool as key.

leocook added 2 commits May 10, 2026 19:27
…ng from upstream to downstream

- Inject VarPool parameters that don't exist in downstream task's local params into prepareParamsMap for placeholder resolution
- Use Matcher.quoteReplacement() in replaceOriginalValue() to prevent $ being treated as regex group reference
- Add null safety check for parameter value before replacement
- Add unit tests for both fixes
…directly putting OUT-direction varPool into params map
@leocook leocook force-pushed the fix-18074-sql-parameter-type-passing branch from 1676161 to 4dbaac0 Compare May 10, 2026 11:35
@leocook
Copy link
Copy Markdown
Contributor Author

leocook commented May 10, 2026

Varpool only be announced as a in parameter then it can be used, we shouldn't directly put varpool as key.

Thanks for the review. Fixed — when injecting a varPool parameter that doesn't exist in local params, a new Property with Direct.IN is now created instead of directly putting the OUT-direction varPool.

@leocook leocook requested a review from ruanwenjun May 10, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Parameter] Exception caused by incorrect parameter type passed from upstream task to downstream task.

2 participants