medkhabt commented on a change in pull request #5556:
URL: https://github.com/apache/dolphinscheduler/pull/5556#discussion_r641971930
##########
File path:
dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java
##########
@@ -495,8 +495,18 @@ public void printReplacedSql(String content, String
formatSql, String rgex, Map<
//parameter print style
logger.info("after replace sql , preparing : {}", formatSql);
StringBuilder logPrint = new StringBuilder("replaced sql ,
parameters:");
- for (int i = 1; i <= sqlParamsMap.size(); i++) {
- logPrint.append(sqlParamsMap.get(i).getValue() + "(" +
sqlParamsMap.get(i).getType() + ")");
+ if(sqlParamsMap == null) {
+ logger.info("printReplacedSql: sqlParamsMap is null.");
+ }
+ else {
+ for (int i = 1; i <= sqlParamsMap.size(); i++) {
+ if(sqlParamsMap.get(i) == null ) {
+ logger.info("printReplacedSql:
sqlParamsMap.get({}) is null.", i);
Review comment:
Hi,
For the friendly logging of a parameter that is null, i should return
'paramName' that wasn't found in `paramsMap` if i understand correctly. To do
so i should probably check if the sql parameter is null in the
`setSqlParamsMap`, as i have that information while setting the `sqlParams`.
Should i still `sqlParamsMap.put(index, prop)` (in `setSqlParamsMap` method)
if the param sql is null. I think i shouldn't put in the first place a null
`Property` in the sqlParamsMap. And as you said the `i` doesn't really
represent anything as the 'Matcher' (that was responsible for setting the `i` )
doesn't order its matches according to its position in the String, correct me
if i'm wrong.
I'll work on these points.
I apologize if my message wasn't well formed, still new to commenting.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
