[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10746: [Feature][SQLTask] Suggest SQL task support variables in anywhere

[GitBox <gi...@apache.org>]

caishunfeng commented on code in PR #10746:
URL: https://github.com/apache/dolphinscheduler/pull/10746#discussion_r945500176


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -341,116 +346,6 @@ private void close(Connection connection) {
         }
     }
 
-    /**
-     * preparedStatement bind
-     *
-     * @param connection connection
-     * @param sqlBinds sqlBinds
-     * @return PreparedStatement
-     * @throws Exception Exception
-     */
-    private PreparedStatement prepareStatementAndBind(Connection connection, SqlBinds sqlBinds) {
-        // is the timeout set
-        boolean timeoutFlag = taskExecutionContext.getTaskTimeoutStrategy() == TaskTimeoutStrategy.FAILED
-                || taskExecutionContext.getTaskTimeoutStrategy() == TaskTimeoutStrategy.WARNFAILED;
-        try {
-            PreparedStatement stmt = connection.prepareStatement(sqlBinds.getSql());
-            if (timeoutFlag) {
-                stmt.setQueryTimeout(taskExecutionContext.getTaskTimeout());
-            }
-            Map<Integer, Property> params = sqlBinds.getParamsMap();
-            if (params != null) {
-                for (Map.Entry<Integer, Property> entry : params.entrySet()) {
-                    Property prop = entry.getValue();
-                    ParameterUtils.setInParameter(entry.getKey(), stmt, prop.getType(), prop.getValue());
-                }
-            }
-            logger.info("prepare statement replace sql : {} ", stmt);
-            return stmt;
-        } catch (Exception exception) {
-            throw new TaskException("SQL task prepareStatementAndBind error", exception);
-        }
-    }
-
-    /**
-     * print replace sql
-     *
-     * @param content content
-     * @param formatSql format sql
-     * @param rgex rgex
-     * @param sqlParamsMap sql params map
-     */
-    private void printReplacedSql(String content, String formatSql, String rgex, Map<Integer, Property> sqlParamsMap) {
-        //parameter print style
-        logger.info("after replace sql , preparing : {}", formatSql);
-        StringBuilder logPrint = new StringBuilder("replaced sql , parameters:");
-        if (sqlParamsMap == null) {
-            logger.info("printReplacedSql: sqlParamsMap is null.");
-        } else {
-            for (int i = 1; i <= sqlParamsMap.size(); i++) {
-                logPrint.append(sqlParamsMap.get(i).getValue()).append("(").append(sqlParamsMap.get(i).getType()).append(")");
-            }
-        }
-        logger.info("Sql Params are {}", logPrint);
-    }
-
-    /**
-     * ready to execute SQL and parameter entity Map
-     *
-     * @return SqlBinds
-     */
-    private SqlBinds getSqlAndSqlParamsMap(String sql) {
-        Map<Integer, Property> sqlParamsMap = new HashMap<>();
-        StringBuilder sqlBuilder = new StringBuilder();
-
-        // combining local and global parameters
-        Map<String, Property> paramsMap = taskExecutionContext.getPrepareParamsMap();
-
-        // spell SQL according to the final user-defined variable
-        if (paramsMap == null) {
-            sqlBuilder.append(sql);
-            return new SqlBinds(sqlBuilder.toString(), sqlParamsMap);
-        }
-
-        if (StringUtils.isNotEmpty(sqlParameters.getTitle())) {
-            String title = ParameterUtils.convertParameterPlaceholders(sqlParameters.getTitle(),
-                    ParamUtils.convert(paramsMap));
-            logger.info("SQL title : {}", title);
-            sqlParameters.setTitle(title);
-        }
-
-        //new
-        //replace variable TIME with $[YYYYmmddd...] in sql when history run job and batch complement job
-        sql = ParameterUtils.replaceScheduleTime(sql, taskExecutionContext.getScheduleTime());
-        // special characters need to be escaped, ${} needs to be escaped
-        setSqlParamsMap(sql, rgex, sqlParamsMap, paramsMap,taskExecutionContext.getTaskInstanceId());
-        //Replace the original value in sql ！{...} ，Does not participate in precompilation
-        String rgexo = "['\"]*\\!\\{(.*?)\\}['\"]*";
-        sql = replaceOriginalValue(sql, rgexo, paramsMap);
-        // replace the ${} of the SQL statement with the Placeholder
-        String formatSql = sql.replaceAll(rgex, "?");
-        // Convert the list parameter
-        formatSql = ParameterUtils.expandListParameter(sqlParamsMap, formatSql);
-        sqlBuilder.append(formatSql);

Review Comment:
   Why remove these logic?



##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -341,116 +346,6 @@ private void close(Connection connection) {
         }
     }
 
-    /**
-     * preparedStatement bind
-     *
-     * @param connection connection
-     * @param sqlBinds sqlBinds
-     * @return PreparedStatement
-     * @throws Exception Exception
-     */
-    private PreparedStatement prepareStatementAndBind(Connection connection, SqlBinds sqlBinds) {
-        // is the timeout set
-        boolean timeoutFlag = taskExecutionContext.getTaskTimeoutStrategy() == TaskTimeoutStrategy.FAILED

Review Comment:
   It seems miss the timeout action.



-- 
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