You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/08/04 06:25:15 UTC
[GitHub] [dolphinscheduler] rickchengx opened a new pull request, #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
rickchengx opened a new pull request, #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
Currently, the log does not output the currently executing sql statement while executing multiple sql statements.
Also, the log output in the `prepareStatementAndBind()` is kind of odd:
For example, as for multiple mysql statements, it outputs
```
[INFO] 2022-08-04 05:56:57.774 +0000 [taskAppId=TASK-20220804-6410011658368_11-73-78] TaskLogLogger-class org.apache.dolphinscheduler.plugin.task.sql.SqlTask:[367] - prepare statement replace sql : HikariProxyPreparedStatement@1694477678 wrapping com.mysql.cj.jdbc.ClientPreparedStatement:
insert into student values ('Ton', 18);
```
which can output the sql statement.
However, as for multiple hive statements, it outputs
```
[INFO] 2022-08-04 05:41:58.610 +0000 [taskAppId=TASK-20220804-6418931330048_7-72-77] TaskLogLogger-class org.apache.dolphinscheduler.plugin.task.sql.SqlTask:[367] - prepare statement replace sql : HikariProxyPreparedStatement@894649176 wrapping org.apache.hive.jdbc.HivePreparedStatement@4041a5d6
```
which can not output the current sql statement since `toString()` is not implemented in the class `HivePreparedStatement`.
So why don't we just delete the log output in the `prepareStatementAndBind()` and add the log output in the `executeUpdate()`
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is code cleanup without any test coverage.
--
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
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r942042729
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
try (PreparedStatement statement = prepareStatementAndBind(connection, sqlBind)) {
result = statement.executeUpdate();
logger.info("{} statement execute update result: {}, for sql: {}", handlerType, result, sqlBind.getSql());
Review Comment:
> I suggest improving the original log in the `prepareStatementAndBind` method, outputting SQL and parameters.
>
> `prepareStatementAndBind` is called in both executeQuery and executeUpdate, and we also need to understand sql and sql parameters.
Thanks for your suggestion. I have added a log output in `prepareStatementAndBind`, outputting SQL and parameters.
--
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
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r956931415
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
try (PreparedStatement statement = prepareStatementAndBind(connection, sqlBind)) {
result = statement.executeUpdate();
logger.info("{} statement execute update result: {}, for sql: {}", handlerType, result, sqlBind.getSql());
Review Comment:
Hi, @zhuangchong, please review 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
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#issuecomment-1204836702
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11292)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#issuecomment-1210207478
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11292)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#issuecomment-1204835459
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11292)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dolphinscheduler] zhuangchong commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
zhuangchong commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r941937616
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
try (PreparedStatement statement = prepareStatementAndBind(connection, sqlBind)) {
result = statement.executeUpdate();
logger.info("{} statement execute update result: {}, for sql: {}", handlerType, result, sqlBind.getSql());
Review Comment:
I suggest improving the original log in the `prepareStatementAndBind` method, outputting SQL and parameters.
`prepareStatementAndBind` is called in both executeQuery and executeUpdate, and we also need to understand sql and sql parameters.
--
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
[GitHub] [dolphinscheduler] zhuangchong commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
zhuangchong commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r941937616
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
try (PreparedStatement statement = prepareStatementAndBind(connection, sqlBind)) {
result = statement.executeUpdate();
logger.info("{} statement execute update result: {}, for sql: {}", handlerType, result, sqlBind.getSql());
Review Comment:
I suggest improving the original log in the `prepareStatementAndBind` method, outputting SQL and parameters.
--
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
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r941063741
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
try (PreparedStatement statement = prepareStatementAndBind(connection, sqlBind)) {
result = statement.executeUpdate();
logger.info("{} statement execute update result: {}, for sql: {}", handlerType, result, sqlBind.getSql());
Review Comment:
> This place has already been printed, do you want to print both sides?
If the sql statement is correct, it will be printed 2 times: once before execution and once after execution gets the result
**However**, if an error occurs in the execution of the sql statement, the log after the execution cannot be printed.
Here is an example: error when executing multiple hive sql statements
<img width="1041" alt="截屏2022-08-09 16 29 59" src="https://user-images.githubusercontent.com/38122586/183603658-aba81485-269a-49c6-9b7f-b981fcc8f08e.png">
And since `toString()` is not implemented in the class `HivePreparedStatement`, the user cannot find which sql statement is executing and throws an exception.
--
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
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#issuecomment-1210206594
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11292)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11292&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11292&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dolphinscheduler] zhuangchong merged pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
zhuangchong merged PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292
--
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
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r940866968
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
Review Comment:
> I don't understand why you did this...
This log outputs the current sql statements while executing multiple sql statements so that user can understand which sql statement is executing. This is helpful when something goes wrong while executing multiple sql statements.
The original log can only output the current sql statement if the sql type is `Mysql`, but cannot output the current sql statement if the sql type is `Hive`.
--
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
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r939769101
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
Review Comment:
I don't understand why you did 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
[GitHub] [dolphinscheduler] zhuangchong commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
zhuangchong commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r940905199
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
try (PreparedStatement statement = prepareStatementAndBind(connection, sqlBind)) {
result = statement.executeUpdate();
logger.info("{} statement execute update result: {}, for sql: {}", handlerType, result, sqlBind.getSql());
Review Comment:
This place has already been printed, do you want to print both sides?
--
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
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#issuecomment-1204830550
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/11292?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#11292](https://codecov.io/gh/apache/dolphinscheduler/pull/11292?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bfa9a4) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/aef2fbf36c76d4dbfcf566ec959acea00e7bec4e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aef2fbf) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## dev #11292 +/- ##
============================================
- Coverage 40.25% 40.22% -0.04%
+ Complexity 4843 4837 -6
============================================
Files 974 974
Lines 37311 37304 -7
Branches 4140 4134 -6
============================================
- Hits 15020 15004 -16
- Misses 20745 20752 +7
- Partials 1546 1548 +2
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/11292?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...erver/master/processor/queue/TaskEventService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza0V2ZW50U2VydmljZS5qYXZh) | `69.64% <0.00%> (-10.72%)` | :arrow_down: |
| [...olphinscheduler/rpc/remote/NettyServerHandler.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcnBjL3JlbW90ZS9OZXR0eVNlcnZlckhhbmRsZXIuamF2YQ==) | `69.04% <0.00%> (-7.15%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `50.00% <0.00%> (-2.78%)` | :arrow_down: |
| [...olphinscheduler/rpc/remote/NettyClientHandler.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcnBjL3JlbW90ZS9OZXR0eUNsaWVudEhhbmRsZXIuamF2YQ==) | `51.16% <0.00%> (-2.33%)` | :arrow_down: |
| [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.96% <0.00%> (-1.64%)` | :arrow_down: |
| [...rver/master/runner/task/BlockingTaskProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvdGFzay9CbG9ja2luZ1Rhc2tQcm9jZXNzb3IuamF2YQ==) | `75.60% <0.00%> (-0.30%)` | :arrow_down: |
| [...eduler/api/service/impl/DataSourceServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhU291cmNlU2VydmljZUltcGwuamF2YQ==) | `28.47% <0.00%> (ø)` | |
| [.../dolphinscheduler/plugin/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...phinscheduler/plugin/task/chunjun/ChunJunTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stY2h1bmp1bi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vdGFzay9jaHVuanVuL0NodW5KdW5UYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...cheduler/plugin/task/api/ShellCommandExecutor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS9TaGVsbENvbW1hbmRFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/dolphinscheduler/pull/11292/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [dolphinscheduler] rickchengx commented on a diff in pull request #11292: [Improvement-11291][sql] Improvement the log output while executing multiple sql statements
Posted by GitBox <gi...@apache.org>.
rickchengx commented on code in PR #11292:
URL: https://github.com/apache/dolphinscheduler/pull/11292#discussion_r964362637
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -302,6 +302,7 @@ private String executeQuery(Connection connection, SqlBinds sqlBinds, String han
private String executeUpdate(Connection connection, List<SqlBinds> statementsBinds, String handlerType) throws Exception {
int result = 0;
for (SqlBinds sqlBind : statementsBinds) {
+ logger.info("{} statement execute update query, for sql: {}", handlerType, sqlBind.getSql());
Review Comment:
Hi, @SbloodyS , please review this.
I have added a log output in prepareStatementAndBind, outputting SQL and parameters.
--
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