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

["leocook (via GitHub)" <gi...@apache.org>]

leocook commented on issue #18074:
URL: https://github.com/apache/dolphinscheduler/issues/18074#issuecomment-4380236402

   I ran into a similar but slightly different parameter passing problem. Also, I noticed the issue uses `${week_of_year}`, but after testing and digging through the code, it should actually be `!{week_of_year}`.
   
   Let me break down the difference between the two parameter formats in SQL tasks (had to check the code and docs to figure this out):
   
   **Format 1: `${param}`**
   - This is a PreparedStatement placeholder
   - Gets replaced with `?`, participates in SQL precompilation
   - Prevents SQL injection, safer
   - Code location: `SqlTask.java:473-474`
   - Mentioned in the official docs
   
   **Format 2: `!{param}`**
   - This is literal value replacement, just shoves the parameter value right in
   - **Does NOT participate in precompilation**
   - Used for scenarios where placeholders don't work, like table names, column names, partition names, etc.
   - Code comment says: `// Replace the original value in sql ！{...} ，Does not participate in precompilation`
   - Code location: `SqlTask.java:475-477`, uses regex `String rgexo = "['\"]*\\!\\{(.*?)\\}['\"]*";`
   
   **My problem scenario:**
   
   - Upstream SQL task: `select 111 as p1`, output parameter is `p1` (OUT direction)
   - Downstream SQL task: input parameter `p2` (IN direction), value is `${p1}`, SQL is `select 222 as pp_!{p2}`
   
   **Error:**
   ```
   java.lang.IllegalArgumentException: No group with name {p1}
       at java.util.regex.Matcher.appendReplacement(Matcher.java:849)
       at java.util.regex.Matcher.replaceFirst(Matcher.java:1004)
       at org.apache.dolphinscheduler.plugin.task.sql.SqlTask.replaceOriginalValue(SqlTask.java:496)
   ```
   
   **Root cause:**
   
   1. VarPool values are only used to override **same-name** parameters. Downstream only has `p2` not `p1`, so the upstream value `p1=111` just gets tossed
   2. So `p2`'s value `${p1}` can't be resolved and stays as `${p1}`
   3. When `replaceOriginalValue` uses this value to replace `!{p2}`, since it doesn't use `Matcher.quoteReplacement()`, the `$` in `${p1}` gets treated as a regex group reference and it crashes
   
   **Side note**: If this issue's example used `!{week_of_year}` instead of `${week_of_year}`, it would probably hit the same problem (because the partition name would contain the string `${week_of_year}` which has `$` in it)
   
   **Fix:**
   
   I've already written a fix:
   1. Inject VarPool parameters into `prepareParamsMap` so parameters with different names can also be referenced
   2. Use `Matcher.quoteReplacement()` for safe replacement to avoid crashes from special characters like `$` and `\`
   
   I'll open a new issue and PR to track this.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org