[Improvement][SQL Task]use default sql segment separator

[zhuxt2015]

Purpose of the pull request

fix #10870

Brief change log

Verify this pull request

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:

[codecov-commenter]

Codecov Report

Merging #10869 (cacd82f) into dev (ada7cf7) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##                dev   #10869      +/-   ##
============================================
- Coverage     39.05%   39.04%   -0.01%     
+ Complexity     4079     4078       -1     
============================================
  Files          1010     1010              
  Lines         37716    37714       -2     
  Branches       4329     4329              
============================================
- Hits          14731    14727       -4     
- Misses        21298    21299       +1     
- Partials       1687     1688       +1     

Impacted Files	Coverage Δ
.../org/apache/dolphinscheduler/spi/enums/DbType.java	0.00% <0.00%> (ø)
...uler/plugin/task/api/parameters/SqlParameters.java	63.00% <ø> (+1.83%)	⬆️
...erver/master/processor/queue/TaskEventService.java	75.00% <0.00%> (-5.36%)	⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java	52.14% <0.00%> (-0.72%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[zhongjiajie]

I think we should better add ;\n as the default value, and users can overwrite it in our web ui. it makes our SQL task more general and does not break the previous behavior. WDYT @zhuangchong it seems you are the function creaetor

[zhuxt2015]

I think we should better add ;\n as the default value, and users can overwrite it in our web ui. it makes our SQL task more general and does not break the previous behavior. WDYT @zhuangchong it seems you are the function creaetor

Can anyone tell me where did the original demand come from? I think it is useless. please remember Less is more.

[zhuangchong]

@zhuxt2015
The original requirement is that the SQL task supports multi-stage SQL execution, for example, the requirement to execute multiple MySQL insert statements in one SQL task.

I have a different opinion from you, and I do not recommend you to modify it in this way. From your PR, you provide a default delimiter in the code. This is no problem when using the MySQL data source, but when using the Hive data source, this delimiter is not needed, Hive itself supports executing multiple segmented statements at a time.

The description of the document I provided is not clear. What I have implemented is

1. The data source that does not support the execution of multiple segmented statements at a time, and I want to execute multiple SQL tasks of multiple segmented statements at a time, at this time, I need to fill in this separator on the page, For example, MySQL, PG
2. Supports data sources that execute multiple segmented statements at a time, without input separators, like Hive

I agree with what you said Less is more, I also think that this delimiter should not be filled in by the user, this parameter is related to the data source, I have expressed my thoughts in PR #9917, if you have time consider implementing

[zhuxt2015]

@zhuangchong
Thanks for the reply.
I made a special judgment about the hive data source, not split hive sql. A more elegant implementation is to add an isSupportMultipleStatement function to the DataSourceClient class. WDYT?

[zhuangchong]

@zhuxt2015
Good idea, I'm not sure that all datasource's statement separator is semicolon, suggestion is optional, and provide default value for this function

[zhuxt2015]

@zhuangchong
I'm sure that all datasource's statement separator is semicolon, it is a part of the ANSI SQL-92 standard. Furthermore, I add isSupportMultipleStatement in DbType instead of DataSourceClient

[zhuxt2015]

@zhuangchong @zhongjiajie PTAL

[zhuangchong]

+1

[zhongjiajie]

@zhuxt2015 it is a incometible change, please also add some description in https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/guide/upgrede/incompatible.md and the ref Chinese doc, thanks

[zhuxt2015]

@zhuxt2015 it is a incometible change, please also add some description in https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/guide/upgrede/incompatible.md and the ref Chinese doc, thanks

ok

[zhongjiajie]

@zhuxt2015 sorry for reply late, and please solve the conflict, thanks

[EricGao888]

LGTM except few nits. PTAL when available, thanks : ) @zhuangchong @zhongjiajie @rickchengx

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[EricGao888]

Backend part LGTM. It would be better if we could have a corresponding UT case.

[EricGao888]

If there are no more comments, I will proceed to merge this PR tomorrow.

[davidzollo]

+1
LGTM, Adding UT case will be better

Backend part LGTM. It would be better if we could have a corresponding UT case.

[zhuangchong]

+1