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/05/20 09:01:29 UTC
[GitHub] [dolphinscheduler] luoyedeyi opened a new pull request, #10170: [Improvement][task sql] Improve creating function format
luoyedeyi opened a new pull request, #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170
<!--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
<!--(For example: This pull request adds checkstyle plugin).-->
## 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.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added dolphinscheduler-dao tests for end-to-end.*
- *Added CronUtilsTest to verify the change.*
- *Manually verified the change by testing locally.* -->
--
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] zhongjiajie commented on pull request #10170: [improve] Using create or replace function in sql task
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1146974133
Hi @SbloodyS , I think the improve change should be 3.1.0 instead of 3.0.0. we should only fix bug in 3.0.0 currently.
--
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 #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1141651894
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10170)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&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 #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1146967500
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10170)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&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] zhongjiajie merged pull request #10170: [improve] Using create or replace function in sql task
Posted by GitBox <gi...@apache.org>.
zhongjiajie merged PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170
--
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] zhongjiajie commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r884396511
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
It is without breaking the before behavior, Am I right? @SbloodyS . I am not really sure after I left the previous comment 🤣
--
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 #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r877940288
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
I think this is unnessesary. It's temporary and won't last forever. If a method is reused, a non temporary method should be created.
--
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 #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r884397315
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
> It is without breaking the before behavior, Am I right? @SbloodyS
Yes. From your point of view, I think it's ok too.
--
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] zhongjiajie commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r889774999
##########
docs/docs/zh/guide/task/sql.md:
##########
@@ -45,4 +45,6 @@ SQL任务类型，用于连接数据库并执行相应SQL。
## 注意事项
-注意SQL类型的选择，如果是INSERT等操作需要选择非查询类型。
\ No newline at end of file
+注意SQL类型的选择，如果是INSERT等操作需要选择非查询类型。
+
+为了兼容长会话情况,UDF函数的创建是通过CREATE OR REPLACE语句
Review Comment:
```suggestion
* 注意SQL类型的选择，如果是INSERT等操作需要选择非查询类型。
* 为了兼容长会话情况,UDF函数的创建是通过CREATE OR REPLACE语句
```
--
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] zhongjiajie commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r884396259
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
I am ok with this patch, it is compatible with more cases without breaking the before behavior, but could we add some comments before L86 to tell developers why should we include `replace` in this syntax?
--
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 #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1132723352
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10170)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10170&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10170&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] luoyedeyi commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
luoyedeyi commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r880004714
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
A long running Spark session will keep its own temp functions instead of destroying them right away. we may get failed message(UDF existed) if we run some sql tasks with UDF. I believe it may be better to change this creating function query.
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
A long running Spark session will keep its own temp functions instead of destroying them right away. we may get failed message(UDF existed) if running some sql tasks with UDF. I believe it may be better to change this creating function query.
--
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] luoyedeyi commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
luoyedeyi commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r880004714
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
A long running Spark session will keep its own temp functions instead of destroying them right away. we may get failed message(UDF existed) while running some sql tasks with UDF. I believe it may be better to change this creating function query.
--
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] luoyedeyi commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
luoyedeyi commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r879996536
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
I believe the temp function could be reused, if we set the value (kyuubi.engine.share.level) to user in Kyuubi. In this case, there is a long running Spark session and all tasks for same user are going to same Spark session. It's more likely to cause sql tasks with UDF to fail, if we only create function.
--
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] zhongjiajie commented on pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1140770140
> Documents need to be submitted at the same time
Good job, although we do not and a new feature on it, we better tell users the inside in our docs https://github.com/apache/dolphinscheduler/tree/dev/docs/docs/en/guide/task, and also the zh doc https://github.com/apache/dolphinscheduler/tree/dev/docs/docs/zh/guide/task
--
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] zhongjiajie commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r884396511
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
It is without breaking the before behavior, Am I right? @SbloodyS
--
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 pull request #10170: [improve] Using create or replace function in sql task
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1146974535
> Hi @SbloodyS , I think the improve change should be 3.1.0 instead of 3.0.0. we should only fix bug in 3.0.0 currently.
Got it. Thanks.
--
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] luoyedeyi commented on pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
luoyedeyi commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1141643486
docs added
--
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] luoyedeyi commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
luoyedeyi commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r880004714
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
A long running Spark session will keep its own temp functions instead of destroying them right away. You may get failed message(UDF existed) if we run some sql task with UDF. I believe it may be better to change this creating function query.
--
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] luoyedeyi commented on a diff in pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
luoyedeyi commented on code in PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#discussion_r880004714
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java:
##########
@@ -83,7 +83,7 @@ public class SqlTask extends AbstractTaskExecutor {
/**
* create function format
*/
- private static final String CREATE_FUNCTION_FORMAT = "create temporary function {0} as ''{1}''";
+ private static final String CREATE_OR_REPLACE_FUNCTION_FORMAT = "create or replace temporary function {0} as ''{1}''";
Review Comment:
A long running Spark session will keep its own temp functions instead of destroying them right away. You may get failed message(UDF existed) if we run some sql tasks with UDF. I believe it may be better to change this creating function query.
--
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 #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1132717172
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10170?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 [#10170](https://codecov.io/gh/apache/dolphinscheduler/pull/10170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (03f92a7) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/c07339b07dea2182bf6a3ba1ff0bcc1fe1f4af55?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c07339b) will **not change** coverage.
> The diff coverage is `n/a`.
> :exclamation: Current head 03f92a7 differs from pull request most recent head 6760b26. Consider uploading reports for the commit 6760b26 to get more accurate results
```diff
@@ Coverage Diff @@
## dev #10170 +/- ##
=========================================
Coverage 40.84% 40.84%
Complexity 4716 4716
=========================================
Files 854 854
Lines 34503 34503
Branches 3814 3814
=========================================
Hits 14093 14093
Misses 19056 19056
Partials 1354 1354
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10170?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10170?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c07339b...6760b26](https://codecov.io/gh/apache/dolphinscheduler/pull/10170?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] songjianet commented on pull request #10170: [Improvement][task sql] Improve creating function query
Posted by GitBox <gi...@apache.org>.
songjianet commented on PR #10170:
URL: https://github.com/apache/dolphinscheduler/pull/10170#issuecomment-1132681720
Please associate the corresponding issue. @luoyedeyi
--
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